Skip to content

Commit f51d6ec

Browse files
sammy-SCfacebook-github-bot
authored andcommitted
fix bug in reparenting where wrong parentTag is used in differentiator (#50884)
Summary: Pull Request resolved: #50884 changelog: [internal] Fix incorrect parentTag coming from differentiator when reparenting. The fix is hidden behind existing feature flag that is already fixing similar issue: D73428312 The problem occurs under very specific circumstances that are covered by an integration test. The *parentTag* is also [only used on Android](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp#L566) and only if layout changes as part of the update. This diff introduces a new test specifically triggering the incorrect behaviour: `Differentiator-itest.js`. Without this fix, the test fails on following assert: [react_native_assert(hasTag(mutation.parentTag))](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactCommon/react/renderer/mounting/stubs/StubViewTree.cpp#L245). Reviewed By: mdvacca Differential Revision: D73545165 fbshipit-source-id: 43344eebfa4cc0119e5b42170f6b9097dacd704d
1 parent d28d4c5 commit f51d6ec

File tree

2 files changed

+62
-1
lines changed

2 files changed

+62
-1
lines changed

packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,11 @@ static void calculateShadowViewMutationsFlattener(
711711

712712
// Unflatten parent, flatten child
713713
if (childReparentMode == ReparentMode::Flatten) {
714+
auto parentTagForUpdateWhenUnflattened =
715+
ReactNativeFeatureFlags::
716+
enableFixForParentTagDuringReparenting()
717+
? newTreeNodePair.shadowView.tag
718+
: parentTag;
714719
// Flatten old tree into new list
715720
// At the end of this loop we still want to know which of these
716721
// children are visited, so we reuse the `newRemainingPairs` map.
@@ -727,7 +732,7 @@ static void calculateShadowViewMutationsFlattener(
727732
oldTreeNodePair,
728733
(reparentMode == ReparentMode::Flatten
729734
? oldTreeNodePair.shadowView.tag
730-
: parentTag),
735+
: parentTagForUpdateWhenUnflattened),
731736
subVisitedNewMap,
732737
subVisitedOldMap,
733738
adjustedOldCullingContext,

packages/react-native/src/private/renderer/mounting/__tests__/Mounting-itest.js

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,62 @@ test('parent-child switching from unflattened-flattened to flattened-unflattened
295295
]);
296296
});
297297

298+
test('parent-child switching from flattened-unflattened to unflattened-flattened', () => {
299+
const root = Fantom.createRoot();
300+
301+
Fantom.runTask(() => {
302+
root.render(
303+
<View
304+
style={{
305+
marginTop: 100,
306+
}}>
307+
<View
308+
style={{
309+
marginTop: 50,
310+
opacity: 0,
311+
}}>
312+
<View nativeID={'child'} style={{height: 10, width: 10}} />
313+
</View>
314+
</View>,
315+
);
316+
});
317+
318+
expect(root.takeMountingManagerLogs()).toEqual([
319+
'Update {type: "RootView", nativeID: (root)}',
320+
'Create {type: "View", nativeID: (N/A)}',
321+
'Create {type: "View", nativeID: "child"}',
322+
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
323+
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
324+
]);
325+
326+
// force view to be flattened.
327+
Fantom.runTask(() => {
328+
root.render(
329+
<View
330+
style={{
331+
marginTop: 100,
332+
opacity: 0,
333+
}}>
334+
<View
335+
style={{
336+
marginTop: 50,
337+
}}>
338+
<View nativeID={'child'} style={{height: 10, width: 10}} />
339+
</View>
340+
</View>,
341+
);
342+
});
343+
expect(root.takeMountingManagerLogs()).toEqual([
344+
'Update {type: "View", nativeID: "child"}',
345+
'Remove {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
346+
'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
347+
'Delete {type: "View", nativeID: (N/A)}',
348+
'Create {type: "View", nativeID: (N/A)}',
349+
'Insert {type: "View", parentNativeID: (root), index: 0, nativeID: (N/A)}',
350+
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "child"}',
351+
]);
352+
});
353+
298354
describe('reconciliation of setNativeProps and React commit', () => {
299355
it('props set by setNativeProps must not be overriden by React commit', () => {
300356
const root = Fantom.createRoot();

0 commit comments

Comments
 (0)