Add unflattenedParentTag to remove mutations

[bartlomiejbloniarz]

Summary:

I am currently implementing Layout Animations for the new architecture in reanimated. One of the issues that I encountered is that view flattening changes the child-parent relationship when views don't form a stacking context. In the current implementation we relay heavily on this relationship in order to prevent parent components from being removed until the child exiting animation ends. To make this work on the new architecture we would have to encourage users to use collapasable: false on non-Animated parent components (or make those components Animated), which could lead to unnecessarily deep trees being created. That's why in this PR I added the unflattenedParentTag property to Remove mutations. This way the unflattened parent-child relationship can be obtained in the MountingOverrideDelegate - which is where Layout Animations are implemented.

Changelog:

[GENERAL] [ADDED] - unflattenedParentTag in ShadowViewNodePair and ShadowViewMutation (only for Remove mutations)
[GENERAL] [CHANGED] - sliceChildShadowNodeViewPairsRecursivelyV2 to store the unflattenedParentTag in ShadowViewNodePairs, so that they can be easily obtained when calculating mutations.

Test Plan:

I used the following example to test the logic in RNTester:

 const [show, setShow] = React.useState(false);

  return (
    <View style={styles.container}>
      <Button title="toggle" onPress={() => setShow(x => !x)} />
      {show && (
        <View>
          <View style={styles.outerBox}>
            <View style={styles.row}>
              <View style={styles.container}>
                <View style={styles.box} />
              </View>
              <View style={styles.container}>
                <View>
                  <View style={styles.box} />
                </View>
              </View>
            </View>
          </View>
        </View>
      )}
    </View>
  );
};

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
    flexDirection: 'column',
  },
  row: {
    flexDirection: 'row',
    justifyContent: 'space-between',
    alignItems: 'center',
  },
  box: {
    width: 50,
    height: 50,
    backgroundColor: 'red',
  },
  outerBox: {
    width: 200,
    height: 100,
    backgroundColor: 'blue',
    justifyContent: 'center',
  },

The example looks like this:

In the debug navigator we can see that red components are actually siblings of the blue component (and also that the layout-only views are not included in the host tree)

But in the remove mutations that were generated when the blue component was removed, we can see that the unflattenedParentTag allows us to see that those red components should be treated as children of the blue component:

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	19,576,611	+8
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	22,946,171	+20
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: a6a7cdf
Branch: main

[facebook-github-bot]

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

[sammy-SC]

Hey,

thanks for the PR. Have you considered any other options other than adding this to remove mutation? If so, how does this solution stack up against them? This information is something that can be detected from mount instructions, why not do that?

If we decide to add something like this to the differentiator, it will be hard to remove it in the future. It would be great to evaluate other options before adding something to the differentiator.

[bartlomiejbloniarz]

Hi @sammy-SC,
thanks for looking into this!

The other approach that I considered, would be to obtain this structure in the commit hook. I would have to calculate mutations using the differentiator, traverse the ShadowTree, looking for deleted views and then store the information about their parents. I think it wouldn't be optimal to calculate mutations twice for every commit (in the commit hook, and then in the Mounting Coordinator). There could also be some issues if there was a commit hook running after our hook.

I think adding this logic to the Differentiator is better, because all the necessary data is there, so there is no need for additional computation of mutations and shadowTree traversal.

I'm not sure how this could be detected from mount instructions? If I understand correctly CREATE and INSERT mutations only operate on the flattened view hierarchy, so this unflattend structure wouldn't be available there. Or do you mean something different by mount instructions?

[react-native-bot]

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[react-native-bot]

This PR was closed because it has been stalled for 7 days with no activity.

[react-native-bot]

This PR was closed because it has been stalled for 7 days with no activity.