-
Notifications
You must be signed in to change notification settings - Fork 168
fix(useTrackerSuspense): resolve unresponsiveness in StrictMode and on query/deps changes #459
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
Conversation
| // To avoid this, check the `timeout` to ensure cleanup only occurs after unmount. | ||
| if (refs.cleanupTimoutId) { | ||
| clearTimeout(refs.cleanupTimoutId) | ||
| delete refs.cleanupTimoutId |
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.
Using the delete operator in V8 (the engine behind Node.js and Chrome) is generally not good for performance, especially in long-running or high-throughput applications.
When you use delete obj.key, V8 must change the object’s internal structure . This invalidates V8’s optimizations and forces the engine to fall back to slower property storage mechanisms .
Set it to undefined instead
refs:
https://phillcode.hashnode.dev/javascript-delete
https://alexanderobregon.substack.com/p/what-happens-when-you-delete-properties
Run this benchmark
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.
👍 Okay, I'll make the changes here
|
|
||
| refs.isMounted = false | ||
| return () => { | ||
| refs.cleanupTimoutId = setTimeout(() => { |
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.
Why settimeout(0) instead of processNextTick?
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.
The code at line 149 is only called by the client. The server directly uses resolveAsync:
export const useTrackerSuspenseServer: typeof useTrackerSuspenseClient = (key, reactiveFn) => {
return resolveAsync(key, Tracker.nonreactive(reactiveFn))
}
Problem Description
This PR fixes two distinct unresponsiveness issues in the
useTrackerSuspensehook:Fixes Included
Commit 1:
fix(useTrackerSuspense): fix unresponsiveness under StrictMode (dev only)Resolved the issue where components don't respond to data changes in React StrictMode (dev only)
Commit 2:
fix(useTrackerSuspense): fix unresponsiveness after query conditions changeFixed the unresponsiveness that occurs after query conditions change (no deps version) or dependencies change (with deps version)
Testing & Verification
Impact