Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor-out usages of Stencil's "readTask" #10348

Closed
1 of 5 tasks
maxpatiiuk opened this issue Sep 19, 2024 · 2 comments
Closed
1 of 5 tasks

Refactor-out usages of Stencil's "readTask" #10348

maxpatiiuk opened this issue Sep 19, 2024 · 2 comments
Assignees
@geospatialem @DitwanP
Labels
4 - verified Issues that have been released and confirmed resolved. calcite-components Issues specific to the @esri/calcite-components package. estimate - 2 Small fix or update, may require updates to tests. p - medium Issue is non core or affecting less that 60% of people using the library refactor Issues tied to code that needs to be significantly reworked.
Milestone
2024-10-29 - Oct ...

Comments

@maxpatiiuk
Copy link
Member

Description

In 3 files, Calcite uses Stencil's readTask() function.

Could you check if there was a particular reason readTask was used or was it just a performance micro optimization?

Lit's default re-render scheduling is very simple - they don't provide readTask/writeTask. However, they expose hook for customizing the scheduling, letting you implement your own alternatives of Stencil's readTask/writeTask if it is important to have them.

Stencil's readTask/writeTask is a smart performance optimization, but it's hardly used - no usages at Esri outside of Calcite, and Calcite has only 3 usages.

TODO:

  • check if readTask() is needed
  • if not, refactor out it's usages

Proposed Advantages

Compatibility with Lit's default scheduling system

Which Component

3 files:

Relevant Info

No response

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-angular
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/eslint-plugin-calcite-components
@maxpatiiuk maxpatiiuk added refactor Issues tied to code that needs to be significantly reworked. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Sep 19, 2024
@github-actions github-actions bot added the calcite-components Issues specific to the @esri/calcite-components package. label Sep 19, 2024
@jcfranco jcfranco mentioned this issue Sep 28, 2024
Merged
@jcfranco jcfranco self-assigned this Sep 30, 2024
@jcfranco jcfranco added 2 - in development Issues that are actively being worked on. estimate - 2 Small fix or update, may require updates to tests. p - medium Issue is non core or affecting less that 60% of people using the library and removed 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Sep 30, 2024
@jcfranco jcfranco added this to the 2024-10-29 - Oct Milestone milestone Sep 30, 2024
jcfranco added a commit that referenced this issue Oct 1, 2024
@jcfranco
refactor: use requestAnimationFrame to replace readTask (#10432)
fe792d3 
**Related Issue:** #10348

## Summary

We've been using `readTask` in a few places to ensure proper timing for
DOM reads w/o triggering layout thrashing. Looking at [the
implementation](https://github.com/ionic-team/stencil/blob/7ede77a873486b5cf47f0b26571f852675f67dd6/src/client/client-task-queue.ts#L23),
we might be able to get close or similar behavior by using
`requestAnimationFrame`.
@jcfranco jcfranco added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Oct 1, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 1, 2024

Installed and assigned for verification.

@github-actions github-actions bot assigned geospatialem and DitwanP and unassigned jcfranco Oct 1, 2024
@DitwanP
Copy link
Contributor

DitwanP commented Oct 2, 2024

🍠✨

@DitwanP DitwanP closed this as completed Oct 2, 2024
@DitwanP DitwanP added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Oct 2, 2024
benelan pushed a commit that referenced this issue Feb 8, 2025
@jcfranco @benelan
refactor: use requestAnimationFrame to replace readTask (#10432)
badd326 
**Related Issue:** #10348

## Summary

We've been using `readTask` in a few places to ensure proper timing for
DOM reads w/o triggering layout thrashing. Looking at [the
implementation](https://github.com/ionic-team/stencil/blob/7ede77a873486b5cf47f0b26571f852675f67dd6/src/client/client-task-queue.ts#L23),
we might be able to get close or similar behavior by using
`requestAnimationFrame`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@geospatialem geospatialem

@DitwanP DitwanP

Labels
4 - verified Issues that have been released and confirmed resolved. calcite-components Issues specific to the @esri/calcite-components package. estimate - 2 Small fix or update, may require updates to tests. p - medium Issue is non core or affecting less that 60% of people using the library refactor Issues tied to code that needs to be significantly reworked.
Projects
None yet
Milestone
2024-10-29 - Oct Milestone
Development

No branches or pull requests
4 participants
@jcfranco @geospatialem @maxpatiiuk @DitwanP