From 8d0b5af1fc13928c024663f10b0257e816bd6696 Mon Sep 17 00:00:00 2001 From: Tomek Zawadzki Date: Tue, 28 Feb 2023 01:28:26 -0800 Subject: [PATCH] Run commit hooks before layout calculation (#36216) 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: https://github.com/facebook/react-native/pull/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 --- ReactCommon/react/renderer/mounting/ShadowTree.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ReactCommon/react/renderer/mounting/ShadowTree.cpp b/ReactCommon/react/renderer/mounting/ShadowTree.cpp index 109cada60db834..0b9106bbb5d2b0 100644 --- a/ReactCommon/react/renderer/mounting/ShadowTree.cpp +++ b/ReactCommon/react/renderer/mounting/ShadowTree.cpp @@ -351,6 +351,10 @@ CommitStatus ShadowTree::tryCommit( } } + // Run commit hooks. + newRootShadowNode = delegate_.shadowTreeWillCommit( + *this, oldRootShadowNode, newRootShadowNode); + // Layout nodes. std::vector affectedLayoutableNodes{}; affectedLayoutableNodes.reserve(1024); @@ -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;