-
Notifications
You must be signed in to change notification settings - Fork 46.5k
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
[WIP] Possible work loop bugfix #22007
base: main
Are you sure you want to change the base?
Conversation
a298d0a
to
6058911
Compare
Comparing: 4225133...7a67816 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
@@ -752,6 +752,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { | |||
return null; | |||
} else { | |||
// Current task was not canceled. Continue. | |||
originalCallbackNode = root.callbackNode; |
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.
What am I missing?
This seems semantically the same since we only set it if root.callbackNode === originalCallbackNode
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.
Sigh you're not missing anything, I pushed the wrong commit 🤦♂️
6058911
to
013d7fe
Compare
I don't think this is the culprit but there's definitely something fishy going on with continuations so I'm going to use this branch to debug |
013d7fe
to
7a67816
Compare
Got a non-reproducible bug report internally. Based on the info in the bug report, I think it might be related to this code here.
The original code is definitely wrong but I don't know if it's the same bug that's being reported.
Pushing this so we can try it and see. Will try to write a test and land the fix regardless.