Preserve state when cloning the shadow tree using cloneTree method

[j-piasecki]

Summary:

The current behavior when calling cloneTree is to not copy the state to the ShadowNodeFragment which results in the state being equal to statePlaceholder. Inside the relevant ShadowNode constructor, the state is checked and if no state was passed, the most recent mounted one is used for the new node:

react-native/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp

Lines 101 to 103 in a1e8118

	state_(
	fragment.state ? fragment.state
	: sourceShadowNode.getMostRecentState()),

mostRecentState is updated during the first construction of a node in the family:

react-native/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp

Line 88 in a1e8118

family_->setMostRecentState(state_);

and when the new node is mounted:

react-native/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp

Lines 282 to 289 in a1e8118

	void ShadowNode::setMounted(bool mounted) const {
	if (mounted) {
	family_->setMostRecentState(getState());
	hasBeenMounted_ = mounted;
	}
	family_->eventEmitter_->setEnabled(mounted);
	}

This turns out to be a problem when using custom commit hooks which rely on the cloneTree method. Let's take the following case:

graph TD
A((A))
B((B))
C((C))
D((D))

A --> B
B --> C
C --> D

Loading

where node B has a custom native state. Now let's say there is also a commit hook that will modify the node D by calling cloneTree with its family as the argument.

In case there is a commit that would modify the native state of B. tryCommit method would be called and would generate a new tree where B has the new state. The commit hook would also trigger on the new tree resulting in the entire tree being cloned to modify the D node in some way. The problem is that it would clone the entire path to D (so A, B, and C) without setting the state in the fragment used to clone them, because of that the state used to clone them the second time would be the most recently committed one. Now, the new state for B node hasn't yet been committed as it happens on mount, so it wouldn't be used when cloning. This effectively means that the state update for a node is dropped when a commit hook tries to modify any of its descendants using the cloneTree method.

Change from this PR makes it so the state from the tree being cloned is used when cloning nodes instead of the most recently committed one. That behavior seems to make more sense to me since potentially modifying the state of nodes on the path to the actually cloned one seems like an unwanted side effect.

The same change has been already rolled out in Reanimated's customized cloning logic (for similar reasons) and we didn't observe any problems caused by it.

Related issue: Expensify/react-native-live-markdown#346

Changelog:

[GENERAL] [FIXED] - Preserve state from the shadow tree being cloned when calling cloneTree

Test Plan:

I've tried it out on RNTestes and didn't see anything out of the ordinary, though I'm not sure whether this change may have some unintended consequences.

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	19,576,603	-98,422
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	22,946,176	-98,346
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: a1e8118
Branch: main

[cortinico]

/rebase

[facebook-github-bot]

@mdvacca has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.