[Persistent] Finalize children after we've actually inserted them (#12300)
The order of this was wrong. We also unconditionally mark for updates so killed that unused branch.
Sebastian Markbåge committed
Feb 26, 2018 at 23:34 UTC
db47031e635faf17d4ed44dfc72779154d2c86ff
4 files changed
+52
-14
packages/react-native-renderer/src/__mocks__/FabricUIManager.js
+18
-10
@@ -14,18 +14,26 @@ const invariant = require('fbjs/lib/invariant');
14
const roots = new Map();
15
const allocatedTags = new Set();
16
17
+function dumpSubtree(info, indent) {
18
+ let out = '';
19
+ out += ' '.repeat(indent) + info.viewName + ' ' + JSON.stringify(info.props);
20
+ // eslint-disable-next-line no-for-of-loops/no-for-of-loops
21
+ for (const child of info.children) {
22
+ out += '\n' + dumpSubtree(child, indent + 2);
23
+ }
24
+ return out;
25
+}
26
+
27
const RCTFabricUIManager = {
18
- __dumpHierarchyForJestTestsOnly: function() {
19
- function dumpSubtree(info, indent) {
20
- let out = '';
21
- out +=
22
- ' '.repeat(indent) + info.viewName + ' ' + JSON.stringify(info.props);
23
- // eslint-disable-next-line no-for-of-loops/no-for-of-loops
24
- for (const child of info.children) {
25
- out += '\n' + dumpSubtree(child, indent + 2);
26
- }
27
- return out;
28
+ __dumpChildSetForJestTestsOnly: function(childSet) {
29
+ let result = [];
30
+ // eslint-disable-next-line no-for-of-loops/no-for-of-loops
31
+ for (const child of childSet) {
32
+ result.push(dumpSubtree(child, 0));
33
}
34
+ return result.join('\n');
35
+ },
36
+ __dumpHierarchyForJestTestsOnly: function() {
37
let result = [];
38
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
39
for (const [rootTag, childSet] of roots) {
packages/react-native-renderer/src/__tests__/ReactFabric-test.internal.js
+25
@@ -247,4 +247,29 @@ describe('ReactFabric', () => {
247
ReactFabric.render(<Component />, 11);
248
expect(mockArgs.length).toEqual(0);
249
});
250
+
251
+ it('should call complete after inserting children', () => {
252
+ const View = createReactNativeComponentClass('View', () => ({
253
+ validAttributes: {foo: true},
254
+ uiViewClassName: 'View',
255
+ }));
256
+
257
+ const snapshots = [];
258
+ FabricUIManager.completeRoot.mockImplementation(function(
259
+ rootTag,
260
+ newChildSet,
261
+ ) {
262
+ snapshots.push(
263
+ FabricUIManager.__dumpChildSetForJestTestsOnly(newChildSet),
264
+ );
265
+ });
266
+
267
+ ReactFabric.render(
268
+ <View foo="a">
269
+ <View foo="b" />
270
+ </View>,
271
+ 22,
272
+ );
273
+ expect(snapshots).toMatchSnapshot();
274
+ });
275
});
packages/react-native-renderer/src/__tests__/__snapshots__/ReactFabric-test.internal.js.snap
+7
@@ -50,6 +50,13 @@ exports[`ReactFabric renders and reorders children 2`] = `
50
View {\\"title\\":\\"y\\"}"
51
`;
52
53
+exports[`ReactFabric should call complete after inserting children 1`] = `
54
+Array [
55
+ "View {\\"foo\\":\\"a\\"}
56
+ View {\\"foo\\":\\"b\\"}",
57
+]
58
+`;
59
+
60
exports[`ReactFabric should only pass props diffs to FabricUIManager.cloneNode 1`] = `
61
"11
62
View {\\"foo\\":\\"a\\",\\"bar\\":\\"b\\"}
packages/react-reconciler/src/ReactFiberCompleteWork.js
+2
-4
@@ -281,14 +281,12 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
281
} else {
282
const container = portalOrRoot.containerInfo;
283
let newChildSet = createContainerChildSet(container);
284
- if (finalizeContainerChildren(container, newChildSet)) {
285
- markUpdate(workInProgress);
286
- }
287
- portalOrRoot.pendingChildren = newChildSet;
284
// If children might have changed, we have to add them all to the set.
285
appendAllChildrenToContainer(newChildSet, workInProgress);
286
+ portalOrRoot.pendingChildren = newChildSet;
287
// Schedule an update on the container to swap out the container.
288
markUpdate(workInProgress);
289
+ finalizeContainerChildren(container, newChildSet);
290
}
291
};
292
updateHostComponent = function(