-
Notifications
You must be signed in to change notification settings - Fork 47k
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] Refactor beginWork replay #14130
Conversation
8f1199b
to
10eddb1
Compare
ReactDOM: size: -0.0%, gzip: -0.0% Details of bundled changes.Comparing: 3c69a18...10eddb1 react-dom
react-art
react-test-renderer
react-reconciler
react-native-renderer
scheduler
Generated by 🚫 dangerJS |
} catch (thrownValue) { | ||
resetContextDependences(); | ||
resetHooks(); | ||
if (nextUnitOfWork === null) { |
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.
I don't think there's any way to hit this condition. nextUnitOfWork
should always be workInProgress
.
@acdlite Do you still care that I follow up on this refactor? |
I addressed this when I rewrote the work loop last year so we can close this |
WIP, doesn’t work
I want to follow up on #14104 (comment) and try to simplify this logic.
So far, I've tried to separate
beginWorkInDEV
that would callbeingWork
and fall back toreplay
in acatch
block. A dozen tests are currently failing with a context mismatch or something like this, but I’m not sure why. I also realized that there might be a reason replaying callsworkLoop
rather thanbeginWork
directly (which seems like it would be simpler?) — for example,beginWork
might assume the profiler timer has already started. So we need to make sure that any initialization that happens prior tobeginWork
also happens on the replay code path, or revert to callingworkLoop
.