Skip to content

Conversation

@yungsters
Copy link
Contributor

Currently, ReactUpdates updates dirty components in increasing order of mount depth. However, mount depth is only relative to the component passed into React.render. This breaks down for components that invoke React.render as an implementation detail because the child components will be updated before the parent component.

This fixes the problem by using the order in which components are mounted (instead of their depth). The mount order transcends component trees (rooted at React.render calls).

Reviewers: @sebmarkbage @zpao

Test Plan:
Ran unit tests successfully:

npm run jest

Currently, `ReactUpdates` updates dirty components in increasing order of mount depth. However, mount depth is only relative to the component passed into `React.render`. This breaks down for components that invoke `React.render` as an implementation detail because the child components will be updated before the parent component.

This fixes the problem by using the order in which components are mounted (instead of their depth). The mount order transcends component trees (rooted at `React.render` calls).

Reviewers: @sebmarkbage @zpao

Test Plan:
Ran unit tests successfully:

```
npm run jest
```
yungsters added a commit that referenced this pull request Nov 19, 2014
Update Dirty Components in Mount Ordering
@yungsters yungsters merged commit 70b1426 into facebook:master Nov 19, 2014
@yungsters yungsters deleted the updates branch November 19, 2014 21:12
yungsters added a commit to yungsters/react that referenced this pull request Nov 19, 2014
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
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants