-
Notifications
You must be signed in to change notification settings - Fork 50.3k
Preserve AsyncLocalStorage context in pingTask using snapshot #35436
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.
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
Closed
feedthejim
wants to merge
5
commits into
facebook:main
from
feedthejim:preserve-async-context-snapshot
Closed
Preserve AsyncLocalStorage context in pingTask using snapshot #35436
feedthejim
wants to merge
5
commits into
facebook:main
from
feedthejim:preserve-async-context-snapshot
+547
−10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e73c02f to
042a634
Compare
609e491 to
7ab1d0a
Compare
When a component suspends on a promise and that promise later resolves, `pingTask()` is called to continue rendering. Previously, `pingTask()` scheduled `performWork` without preserving the full async context that was active when the request started. This caused third-party AsyncLocalStorage contexts (like Next.js's `workUnitAsyncStorage`) to be lost when rendering continued after promise resolution. This fix uses `AsyncLocalStorage.snapshot()` (available in Node.js 18.2.0+) to capture the entire async context stack when `startWork()` begins, and restores it when `pingTask()` schedules continuation work. The `snapshot()` API captures ALL async contexts in the stack, not just React's own `requestStorage`. This ensures that frameworks like Next.js can maintain their own AsyncLocalStorage contexts across React's internal scheduling boundaries. Changes: - Added `asyncContextSnapshot` field to Request type - Added `createAsyncContextSnapshot()` to config forks - Modified `startWork()` to capture the snapshot - Modified `pingTask()` to restore the snapshot when scheduling work - Added test to verify third-party AsyncLocalStorage context is preserved 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
7ab1d0a to
9bfa965
Compare
- Add supportsRequestStorage check to pingTask conditions to prevent calling requestStorage.run() when requestStorage is null (browser env) - Add createAsyncContextSnapshot to noop renderer configs for tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Null out the snapshot when the request is CLOSED to allow earlier garbage collection of captured AsyncLocalStorage contexts. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Verifies that async context snapshots are properly cleaned up between requests, preventing context leakage from one request to another. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Moved snapshot capture inside requestStorage.run() so the snapshot includes both third-party AsyncLocalStorage contexts AND React's own requestStorage context. This ensures correct context nesting when restored in pingTask. Also simplified pingTask to not need nested requestStorage.run() when restoring from snapshot, since requestStorage is now part of the captured context. Added tests for: - Macrotask boundary context preservation - Nested AsyncLocalStorage contexts
0cb9b72 to
3f4b9d0
Compare
unstubbable
added a commit
to unstubbable/react
that referenced
this pull request
Jan 5, 2026
lubieowoce
reviewed
Jan 5, 2026
Comment on lines
+720
to
+723
| if (!hasLoaded) { | ||
| contextValueDuringRender = thirdPartyStorage.getStore(); | ||
| throw promise; | ||
| } |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should probably just use(promise)?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
When a component suspends on a promise and that promise later resolves,
pingTask()is called to continue rendering. Previously,pingTask()scheduledperformWorkwithout preserving the full async context that was active when the request started.This caused third-party AsyncLocalStorage contexts (like Next.js's
workUnitAsyncStorage) to be lost when rendering continued after promise resolution, leading to flaky test failures in Next.js: vercel/next.js#76066The Problem
When a promise resolves after React's initial
performWorkhas returned,pingTask()schedules a NEWperformWorkin a fresh microtask/macrotask. This new execution context doesn't have the original AsyncLocalStorage contexts.The Fix
This PR uses
AsyncLocalStorage.snapshot()(available in Node.js 18.2.0+) to capture the entire async context stack and restore it whenpingTask()schedules continuation work.Key insight: The snapshot must be captured inside
requestStorage.run()so it includes both:workUnitAsyncStorage)requestStoragecontextThis ensures correct context nesting when restored, and simplifies
pingTask()sincerequestStorageis already part of the captured snapshot.Changes
asyncContextSnapshotfield to Request type in both Fizz and FlightcreateAsyncContextSnapshot()to all config forks (Node, Edge, Browser, Noop)startWork()to capture snapshot insiderequestStorage.run()pingTask()to restore snapshot directly (no nestedrequestStorage.run()needed)asyncContextSnapshot = nullwhen request closesHow did you test this change?
Added tests in
ReactDOMFizzServerNode-test.js:should preserve third-party AsyncLocalStorage context after promise resolutionshould preserve third-party AsyncLocalStorage context across macrotask boundariesshould preserve nested AsyncLocalStorage contexts with correct nesting ordershould not leak async context between requests after cleanupAll tests verify that
thirdPartyStorage.getStore()returns the expected value after suspension/resumption.