-
-
Notifications
You must be signed in to change notification settings - Fork 93
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
Use suspense boundary vnode as parent for subtree re-render #408
Conversation
🦋 Changeset detectedLatest commit: 2022f10 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
This does look good, we'll need a changeset and a few tests to prevent regressions. You might need to update the types of the renderer as well
Would this be a |
c4f90dc
to
28d27a5
Compare
Added changeset, updated types, and added a single test that traverses VNode parents inside the suspense boundary and checks if there is a circular reference. |
You should do the useId test and leverage the abortController to abort after i.e 1 second and fail |
I don't think that's possible either? The synchronous while loop inside the useId hook completely blocks the Node.js event loop, so as far as I know there is no way to abort the abortController while it's in the loop; timeouts/intervals won't even be called. Tripwire as mentioned in the mocha issue uses a native extension to be able to interrupt unbreakable loops, but including a native extension + build step doesn't seem ideal either. Perhaps it would be good to add a check in while loops that could otherwise run forever on erroneous node trees; so far I found this in useId, and the _catchError handler, though that's probably a performance tradeoff |
I mean, it works now so we should not care about a test loop.... we should write a test, i can write it if you prefer. |
This test also expects the current behavior where |
Fixes #406