Skip to content

track inProgressNodes to prevent duplicate async processes#693

Merged
tmarmer merged 1 commit intomainfrom
async-promise-tracking
Jul 30, 2025
Merged

track inProgressNodes to prevent duplicate async processes#693
tmarmer merged 1 commit intomainfrom
async-promise-tracking

Conversation

@tmarmer
Copy link
Copy Markdown
Contributor

@tmarmer tmarmer commented Jul 28, 2025

Change Type (required)

Indicate the type of change your pull request is:

  • patch
  • minor
  • major
  • N/A

Does your PR have any documentation updates?

  • Updated docs
  • No Update needed
  • Unable to update docs

Release Notes

  • fix an issue where promises generated by onAsyncNode could be generated multiple times if a view update caused the same async node to get resolved again before the promise completed.

@tmarmer tmarmer requested a review from a team as a code owner July 28, 2025 17:58
@tmarmer tmarmer force-pushed the async-promise-tracking branch from 557021e to 51cc75a Compare July 29, 2025 14:32
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.12%. Comparing base (111c927) to head (51cc75a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #693   +/-   ##
=======================================
  Coverage   90.11%   90.12%           
=======================================
  Files         333      333           
  Lines       21122    21131    +9     
  Branches     2107     2109    +2     
=======================================
+ Hits        19035    19045   +10     
+ Misses       2069     2068    -1     
  Partials       18       18           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment on lines +285 to +286
nodeResolveCache: new Map(),
inProgressNodes: new Set(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we reset these when the view transitions just to avoid any potential issues with stale data from a previous view?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that was the intention here with setting up the context object. Because this is a ViewPlugin the apply gets called on each new ViewInstance so each cache should be scoped to the current view and be reinitialized on the next view state.

@tmarmer tmarmer merged commit 0bd0ae2 into main Jul 30, 2025
12 checks passed
@tmarmer tmarmer deleted the async-promise-tracking branch July 30, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants