Support passing in children directly when cloning nodes #39817
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request was exported from Phabricator. Differential Revision: D49912532 |
Base commit: 1719a07 |
|
This pull request was exported from Phabricator. Differential Revision: D49912532 |
452b95d to
a574ce3
Compare
javache
added a commit
to javache/react-native
that referenced
this pull request
Oct 5, 2023
Summary: Update the UIManagerBinding interface to support the upcoming API changes in facebook/react#27458 Changelog: [Internal] Differential Revision: D49912532
|
This pull request was exported from Phabricator. Differential Revision: D49912532 |
1 similar comment
|
This pull request was exported from Phabricator. Differential Revision: D49912532 |
javache
added a commit
to javache/react-native
that referenced
this pull request
Oct 6, 2023
Summary: Update the UIManagerBinding interface to support the upcoming API changes in facebook/react#27458 Changelog: [Internal] Reviewed By: rubennorte, sammy-SC Differential Revision: D49912532
8e7abcd to
5261097
Compare
javache
added a commit
to javache/react-native
that referenced
this pull request
Oct 6, 2023
Summary: Update the UIManagerBinding interface to support the upcoming API changes in facebook/react#27458 Changelog: [Internal] Reviewed By: rubennorte, sammy-SC Differential Revision: D49912532
|
This pull request was exported from Phabricator. Differential Revision: D49912532 |
javache
added a commit
to javache/react-native
that referenced
this pull request
Oct 6, 2023
Summary: Update the UIManagerBinding interface to support the upcoming API changes in facebook/react#27458 Changelog: [Internal] Reviewed By: rubennorte, sammy-SC Differential Revision: D49912532
5261097 to
0c0abc5
Compare
javache
added a commit
to javache/react-native
that referenced
this pull request
Oct 6, 2023
Summary: Update the UIManagerBinding interface to support the upcoming API changes in facebook/react#27458 Changelog: [Internal] Reviewed By: rubennorte, sammy-SC Differential Revision: D49912532
0c0abc5 to
36bce10
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49912532 |
javache
added a commit
to javache/react-native
that referenced
this pull request
Oct 6, 2023
Summary: Update the UIManagerBinding interface to support the upcoming API changes in facebook/react#27458 Changelog: [Internal] Reviewed By: rubennorte, sammy-SC Differential Revision: D49912532
36bce10 to
b9ce925
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49912532 |
b9ce925 to
c13e0c8
Compare
javache
added a commit
to javache/react-native
that referenced
this pull request
Oct 6, 2023
Summary: Update the UIManagerBinding interface to support the upcoming API changes in facebook/react#27458 Changelog: [Internal] Reviewed By: rubennorte, sammy-SC Differential Revision: D49912532
|
This pull request was exported from Phabricator. Differential Revision: D49912532 |
Summary: Update the UIManagerBinding interface to support the upcoming API changes in facebook/react#27458 Changelog: [Internal] Reviewed By: rubennorte, sammy-SC Differential Revision: D49912532
|
This pull request was exported from Phabricator. Differential Revision: D49912532 |
c13e0c8 to
e686de3
Compare
|
This pull request was successfully merged by @javache in 239079e. When will my fix make it into a release? | Upcoming Releases |
|
This pull request has been merged in 239079e. |
javache
added a commit
to facebook/react
that referenced
this pull request
Oct 10, 2023
## Summary Currently when cloning nodes in Fabric, we reset a node's children on each clone, and then repeatedly call appendChild to restore the previous list of children (even if it was quasi-identical to before). This causes unnecessary invalidation of the layout state in Fabric's ShadowNode data (which in turn may require additional yoga clones) and extra JSI calls. This PR adds a feature flag to pass in the children as part of the clone call, so Fabric always has a complete view of the node that's being mutated. This feature flag requires matching changes in the react-native repo: facebook/react-native#39817 ## How did you test this change? Unit test added demonstrates the new behaviour ``` yarn test -r www-modern ReactFabric-test yarn test ReactFabric-test.internal ``` Tested a manual sync into React Native and verified core surfaces render correctly.
github-actions bot
pushed a commit
to facebook/react
that referenced
this pull request
Oct 10, 2023
## Summary Currently when cloning nodes in Fabric, we reset a node's children on each clone, and then repeatedly call appendChild to restore the previous list of children (even if it was quasi-identical to before). This causes unnecessary invalidation of the layout state in Fabric's ShadowNode data (which in turn may require additional yoga clones) and extra JSI calls. This PR adds a feature flag to pass in the children as part of the clone call, so Fabric always has a complete view of the node that's being mutated. This feature flag requires matching changes in the react-native repo: facebook/react-native#39817 ## How did you test this change? Unit test added demonstrates the new behaviour ``` yarn test -r www-modern ReactFabric-test yarn test ReactFabric-test.internal ``` Tested a manual sync into React Native and verified core surfaces render correctly. DiffTrain build for [151e75a](151e75a)
alunyov
pushed a commit
to alunyov/react
that referenced
this pull request
Oct 11, 2023
## Summary Currently when cloning nodes in Fabric, we reset a node's children on each clone, and then repeatedly call appendChild to restore the previous list of children (even if it was quasi-identical to before). This causes unnecessary invalidation of the layout state in Fabric's ShadowNode data (which in turn may require additional yoga clones) and extra JSI calls. This PR adds a feature flag to pass in the children as part of the clone call, so Fabric always has a complete view of the node that's being mutated. This feature flag requires matching changes in the react-native repo: facebook/react-native#39817 ## How did you test this change? Unit test added demonstrates the new behaviour ``` yarn test -r www-modern ReactFabric-test yarn test ReactFabric-test.internal ``` Tested a manual sync into React Native and verified core surfaces render correctly.
jerrydev0927
added a commit
to jerrydev0927/react
that referenced
this pull request
Jan 5, 2024
## Summary Currently when cloning nodes in Fabric, we reset a node's children on each clone, and then repeatedly call appendChild to restore the previous list of children (even if it was quasi-identical to before). This causes unnecessary invalidation of the layout state in Fabric's ShadowNode data (which in turn may require additional yoga clones) and extra JSI calls. This PR adds a feature flag to pass in the children as part of the clone call, so Fabric always has a complete view of the node that's being mutated. This feature flag requires matching changes in the react-native repo: facebook/react-native#39817 ## How did you test this change? Unit test added demonstrates the new behaviour ``` yarn test -r www-modern ReactFabric-test yarn test ReactFabric-test.internal ``` Tested a manual sync into React Native and verified core surfaces render correctly. DiffTrain build for [151e75a128d0fd436dce365335b96c5686f704d4](facebook/react@151e75a)
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
## Summary Currently when cloning nodes in Fabric, we reset a node's children on each clone, and then repeatedly call appendChild to restore the previous list of children (even if it was quasi-identical to before). This causes unnecessary invalidation of the layout state in Fabric's ShadowNode data (which in turn may require additional yoga clones) and extra JSI calls. This PR adds a feature flag to pass in the children as part of the clone call, so Fabric always has a complete view of the node that's being mutated. This feature flag requires matching changes in the react-native repo: facebook/react-native#39817 ## How did you test this change? Unit test added demonstrates the new behaviour ``` yarn test -r www-modern ReactFabric-test yarn test ReactFabric-test.internal ``` Tested a manual sync into React Native and verified core surfaces render correctly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update the UIManagerBinding interface to support the upcoming API changes in facebook/react#27458
Changelog: [Internal]
Differential Revision: D49912532