Repository navigation
Fix Suspense and Context from colliding for suspending boundaries - #5303
Open
jacob-ebey wants to merge 3 commits into
Open
jacob-ebey wants to merge 3 commits into
jacob-ebey wants to merge 3 commits into
Conversation
rschristian
reviewed
Oct 9, 2026
| "$_vnode": "__v", | ||
| "$_children": "__k", | ||
| "$_pendingSuspensionCount": "__u", | ||
| "$_childDidSuspend": "__c", |
Member
There was a problem hiding this comment.
If anything, the other properties will need to be the ones changed; childDidSuspend is used in other repos and changing it would be a big break.
Member
There was a problem hiding this comment.
We should just change the _id, which is the only possible collision when you use context as a component.
- Keep _childDidSuspend mangled as __c: other repos (e.g. preact-iso's router) rely on that name, so renaming it would be a breaking change. - Give _id (the identifier on every context object) a unique mangled name (__I) instead. It is the only other __c property that can realistically appear on a component in the Suspense boundary walk (when a context object is used as a component); _cleanup, _component and _commit never do. - Rework the regression test to target the actual cause: it now uses the real _id from a real createContext() call instead of a synthetic string, and drops the _cleanup case (impossible in practice). Co-authored-by: jacob-test-profile <jacob-test-profile@users.noreply.github.com>
jacob-ebey
marked this pull request as ready for review
October 9, 2026 18:59
JoviDeCroock
changed the title
JoviDeCroock
approved these changes
Oct 10, 2026
|
coverage: 99.857%. remained the same — jacob-ebey:suspense-retry into preactjs:main |
JoviDeCroock
requested changes
Oct 10, 2026
JoviDeCroock
left a comment
JoviDeCroock
left a comment
Member
There was a problem hiding this comment.
When I saw that preact-render-to-string uses _id I decided to try and reproduce this and to my surprise I could not. Even the test hacks this by doing this._id = Ctx._id. I think the agent coding the test got confused between the constructor and the instance
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Fixes a Suspense boundary minification collision. mangle.json mapped _id (the identifier on every context object from createContext) to the same minified name (__c) as Suspense's _childDidSuspend.
In built output, the compat _catchError hook finds the nearest boundary by walking up the vnode parents and checking component._childDidSuspend, which becomes component.__c after minification. A component carrying a context's _id, for example when a context object is used as a component, falsely matches. The thrown promise then goes to the wrong component and the real boundary never retries after it resolves.