-
Notifications
You must be signed in to change notification settings - Fork 48.8k
Move beforeblur phase to prepareForCommit #18609
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
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 9301d38:
|
Details of bundled changes.Comparing: b928fc0...9301d38 react-dom
react-reconciler
ReactDOM: size: 0.0%, gzip: 0.0% Size changes (stable) |
Details of bundled changes.Comparing: b928fc0...9301d38 react-dom
react-reconciler
Size changes (experimental) |
c51a3d3
to
3e4bb27
Compare
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 have a ton of context on this but the change seems okay.
fiber.memoizedState = null; | ||
fiber.pendingProps = null; | ||
fiber.return = null; | ||
fiber.sibling = 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.
This seems like a nice change 👍
@@ -95,6 +94,10 @@ import { | |||
import {getListenerMapForElement} from '../events/DOMEventListenerMap'; | |||
import {TOP_BEFORE_BLUR, TOP_AFTER_BLUR} from '../events/DOMTopLevelEventTypes'; | |||
|
|||
// TODO: This is an exposed internal, we should move this around | |||
// so this isn't the case. | |||
import {isFiberInsideHiddenOrRemovedTree} from 'react-reconciler/src/ReactFiberTreeReflection'; |
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.
cc @acdlite in case this affects your fork. I don't think it will.
Add comment Add comment
f2d1396
to
9301d38
Compare
This is a follow up to #18564. Specifically, this moves the
beforeblur
logic inReactDOMHostConfig
, so that it occurs duringprepareForCommit
– avoiding the issues with conflicting with a partially mutated tree. To do this logic we pass through a helper function toprepareForCommit
calledisInstanceInsideHiddenOrRemovedTree
. This helps us determine if the "active element" is within a sub-tree that is either hidden or removed.Furthermore, the effeciency of this approach should be a nice improvement, as it means we have to do far less logic than before – along with a slight code size improvement.
I also applied the
detachFiber
changes from #18564 and included the regression test from that PR too.