-
Notifications
You must be signed in to change notification settings - Fork 46.4k
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
Replace mountDepth
with isTopLevel
#2571
Conversation
Summary: After facebook#2570, `mountDepth` is only used to enforce that `setProps` and `replaceProps` is only invoked on the top-level component. This replaces `mountDepth` with a simpler `isTopLevel` boolean set by `ReactMount` which reduces the surface area of the internal API and removes the need to thread `mountDepth` throughout React core. Reviewers: @sebmarkbage @zpao Test Plan: Ran unit tests successfully: ``` npm run jest ```
@@ -225,7 +225,8 @@ function mountComponentIntoNode( | |||
container, | |||
transaction, | |||
shouldReuseMarkup) { | |||
var markup = this.mountComponent(rootID, transaction, 0, emptyObject); | |||
var markup = this.mountComponent(rootID, transaction, emptyObject); | |||
this._isTopLevel = true; |
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.
Is it safe here to assume that this
is going to be a ReactCompositeComponent
?
Edit: Otherwise, preventExtension
will complain. 👎
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.
Yea, it's because the callback is used with a "context". Annoying OO pattern.
Edit: This will probably change but it's currently true.
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, thanks!
Replace `mountDepth` with `isTopLevel`
Summary:
After #2570,
mountDepth
is only used to enforce thatsetProps
andreplaceProps
is only invoked on the top-level component. This replacesmountDepth
with a simpler
isTopLevel
boolean set byReactMount
which reduces the surface area of the internal API and removes the need to threadmountDepth
throughout React core.Reviewers: @sebmarkbage @zpao
Test Plan:
Ran unit tests successfully: