Skip to content

Commit

Permalink
Run commit hooks before layout calculation (#36216)
Browse files Browse the repository at this point in the history
Summary:
I've noticed that `UIManagerCommitHooks` are applied after calling `layoutIfNeeded`, meaning that if a commit hook makes some changes to the tree, it needs to calculate the layout again, effectively making the first calculation redundant.

This PR swaps the order of these two operations and moves `shadowTreeWillCommit` call before `layoutIfNeeded`. Thanks to this change, commit hooks don't need to manually trigger layout calculations as well as can potentially operate on unsealed nodes which can make it more efficient in some cases.

Finally, this PR eliminates a crash on `emitLayoutEvents(affectedLayoutableNodes);` when commit hook actually modifies the tree and thus de-allocates old shadow nodes.

cc sammy-SC

## Changelog

[GENERAL] [CHANGED] - Run commit hooks before layout calculation

Pull Request resolved: #36216

Test Plan: The only `UIManagerCommitHook` I could find in the OSS repo is [`TimelineController`](https://github.com/facebook/react-native/blob/8bd3edec88148d0ab1f225d2119435681fbbba33/ReactCommon/react/renderer/timeline/TimelineController.h#L26).

Reviewed By: cipolleschi

Differential Revision: D43569407

Pulled By: sammy-SC

fbshipit-source-id: 9ab1de0954cac2eb00346be7af8c9b3572bd2c88
  • Loading branch information
tomekzaw authored and facebook-github-bot committed Feb 28, 2023
1 parent 987dd6a commit 8d0b5af
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions ReactCommon/react/renderer/mounting/ShadowTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,10 @@ CommitStatus ShadowTree::tryCommit(
}
}

// Run commit hooks.
newRootShadowNode = delegate_.shadowTreeWillCommit(
*this, oldRootShadowNode, newRootShadowNode);

// Layout nodes.
std::vector<LayoutableShadowNode const *> affectedLayoutableNodes{};
affectedLayoutableNodes.reserve(1024);
Expand All @@ -374,9 +378,6 @@ CommitStatus ShadowTree::tryCommit(

auto newRevisionNumber = oldRevision.number + 1;

newRootShadowNode = delegate_.shadowTreeWillCommit(
*this, oldRootShadowNode, newRootShadowNode);

if (!newRootShadowNode ||
(commitOptions.shouldYield && commitOptions.shouldYield())) {
return CommitStatus::Cancelled;
Expand Down

0 comments on commit 8d0b5af

Please sign in to comment.