Fiber: Fix reentrant mounting in synchronous mode (#8623)
Ben Alpert committed
Dec 22, 2016 at 11:34 UTC
13980e6536dbd36b13987e3ef60fac3baffb2df3
9 files changed
+71
-37
scripts/fiber/tests-passing.txt
+1
@@ -1568,6 +1568,7 @@ src/renderers/shared/shared/__tests__/ReactUpdates-test.js
1568
* does not update one component twice in a batch (#6371)
1569
* unstable_batchedUpdates should return value from a callback
1570
* unmounts and remounts a root in the same batch
1571
+* handles reentrant mounting in synchronous mode
1572
1573
src/renderers/shared/shared/__tests__/refs-destruction-test.js
1574
* should remove refs when destroying the parent
src/renderers/art/ReactARTFiber.js
+3
-2
@@ -328,9 +328,10 @@ class Surface extends Component {
328
329
this._surface = Mode.Surface(+width, +height, this._tagRef);
330
331
- this._mountNode = ARTRenderer.mountContainer(
331
+ this._mountNode = ARTRenderer.createContainer(this._surface);
332
+ ARTRenderer.updateContainer(
333
this.props.children,
333
- this._surface,
334
+ this._mountNode,
335
this,
336
);
337
}
src/renderers/dom/fiber/ReactDOMFiber.js
+4
-5
@@ -289,16 +289,15 @@ function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any,
289
290
let container : DOMContainerElement =
291
containerNode.nodeType === DOCUMENT_NODE ? (containerNode : any).documentElement : (containerNode : any);
292
- let root;
293
- if (!container._reactRootContainer) {
292
+ let root = container._reactRootContainer;
293
+ if (!root) {
294
// First clear any existing content.
295
while (container.lastChild) {
296
container.removeChild(container.lastChild);
297
}
298
- root = container._reactRootContainer = DOMRenderer.mountContainer(children, container, parentComponent, callback);
299
- } else {
300
- DOMRenderer.updateContainer(children, root = container._reactRootContainer, parentComponent, callback);
298
+ root = container._reactRootContainer = DOMRenderer.createContainer(container);
299
}
300
+ DOMRenderer.updateContainer(children, root, parentComponent, callback);
301
return DOMRenderer.getPublicRootInstance(root);
302
}
303
src/renderers/native/ReactNativeFiber.js
+2
-4
@@ -361,12 +361,10 @@ const ReactNative = {
361
if (!root) {
362
// TODO (bvaughn): If we decide to keep the wrapper component,
363
// We could create a wrapper for containerTag as well to reduce special casing.
364
- root = NativeRenderer.mountContainer(element, containerTag, null, callback);
365
-
364
+ root = NativeRenderer.createContainer(containerTag);
365
roots.set(containerTag, root);
367
- } else {
368
- NativeRenderer.updateContainer(element, root, null, callback);
366
}
367
+ NativeRenderer.updateContainer(element, root, null, callback);
368
369
return NativeRenderer.getPublicRootInstance(root);
370
},
src/renderers/noop/ReactNoop.js
+4
-7
@@ -188,17 +188,14 @@ var ReactNoop = {
188
},
189
190
renderToRootWithID(element : ReactElement<any>, rootID : string, callback : ?Function) {
191
- if (!roots.has(rootID)) {
191
+ let root = roots.get(rootID);
192
+ if (!root) {
193
const container = { rootID: rootID, children: [] };
194
rootContainers.set(rootID, container);
194
- const root = NoopRenderer.mountContainer(element, container, null, callback);
195
+ root = NoopRenderer.createContainer(container);
196
roots.set(rootID, root);
196
- } else {
197
- const root = roots.get(rootID);
198
- if (root) {
199
- NoopRenderer.updateContainer(element, root, null, callback);
200
- }
197
}
198
+ NoopRenderer.updateContainer(element, root, null, callback);
199
},
200
201
unmountRootWithID(rootID : string) {
src/renderers/shared/fiber/ReactFiberBeginWork.js
+2
-1
@@ -280,7 +280,8 @@ module.exports = function<T, P, I, TI, C, CX>(
280
root.pendingContext,
281
root.pendingContext !== root.context
282
);
283
- } else {
283
+ } else if (root.context) {
284
+ // Should always be set
285
pushTopLevelContextObject(
286
workInProgress,
287
root.context,
src/renderers/shared/fiber/ReactFiberReconciler.js
+15
-15
@@ -75,7 +75,7 @@ export type HostConfig<T, P, I, TI, C, CX> = {
75
};
76
77
export type Reconciler<C, I, TI> = {
78
- mountContainer(element : ReactNodeList, containerInfo : C, parentComponent : ?ReactComponent<any, any, any>) : OpaqueNode,
78
+ createContainer(containerInfo : C) : OpaqueNode,
79
updateContainer(element : ReactNodeList, container : OpaqueNode, parentComponent : ?ReactComponent<any, any, any>) : void,
80
performWithPriority(priorityLevel : PriorityLevel, fn : Function) : void,
81
/* eslint-disable no-undef */
@@ -119,17 +119,10 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
119
120
return {
121
122
- mountContainer(element : ReactNodeList, containerInfo : C, parentComponent : ?ReactComponent<any, any, any>, callback: ?Function) : OpaqueNode {
123
- const context = getContextForSubtree(parentComponent);
124
- const root = createFiberRoot(containerInfo, context);
122
+ createContainer(containerInfo : C) : OpaqueNode {
123
+ const root = createFiberRoot(containerInfo);
124
const current = root.current;
125
127
- scheduleTopLevelUpdate(current, element, callback);
128
-
129
- if (__DEV__ && ReactFiberInstrumentation.debugTool) {
130
- ReactFiberInstrumentation.debugTool.onMountContainer(root);
131
- }
132
-
126
// It may seem strange that we don't return the root here, but that will
127
// allow us to have containers that are in the middle of the tree instead
128
// of being roots.
@@ -141,19 +134,26 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
134
const root : FiberRoot = (container.stateNode : any);
135
const current = root.current;
136
144
- root.pendingContext = getContextForSubtree(parentComponent);
145
-
146
- scheduleTopLevelUpdate(current, element, callback);
147
-
137
if (__DEV__) {
138
if (ReactFiberInstrumentation.debugTool) {
150
- if (element === null) {
139
+ if (current.alternate === null) {
140
+ ReactFiberInstrumentation.debugTool.onMountContainer(root);
141
+ } else if (element === null) {
142
ReactFiberInstrumentation.debugTool.onUnmountContainer(root);
143
} else {
144
ReactFiberInstrumentation.debugTool.onUpdateContainer(root);
145
}
146
}
147
}
148
+
149
+ const context = getContextForSubtree(parentComponent);
150
+ if (root.context === null) {
151
+ root.context = context;
152
+ } else {
153
+ root.pendingContext = context;
154
+ }
155
+
156
+ scheduleTopLevelUpdate(current, element, context, callback);
157
},
158
159
performWithPriority,
src/renderers/shared/fiber/ReactFiberRoot.js
+3
-3
@@ -26,11 +26,11 @@ export type FiberRoot = {
26
// The work schedule is a linked list.
27
nextScheduledRoot: ?FiberRoot,
28
// Top context object, used by renderSubtreeIntoContainer
29
- context: Object,
29
+ context: ?Object,
30
pendingContext: ?Object,
31
};
32
33
-exports.createFiberRoot = function(containerInfo : any, context : Object) : FiberRoot {
33
+exports.createFiberRoot = function(containerInfo : any) : FiberRoot {
34
// Cyclic construction. This cheats the type system right now because
35
// stateNode is any.
36
const uninitializedFiber = createHostRootFiber();
@@ -40,7 +40,7 @@ exports.createFiberRoot = function(containerInfo : any, context : Object) : Fibe
40
isScheduled: false,
41
nextScheduledRoot: null,
42
callbackList: null,
43
- context: context,
43
+ context: null,
44
pendingContext: null,
45
};
46
uninitializedFiber.stateNode = root;
src/renderers/shared/shared/__tests__/ReactUpdates-test.js
+37
@@ -1105,4 +1105,41 @@ describe('ReactUpdates', () => {
1105
});
1106
expect(container.textContent).toBe('b');
1107
});
1108
+
1109
+ it('handles reentrant mounting in synchronous mode', () => {
1110
+ var mounts = 0;
1111
+ class Editor extends React.Component {
1112
+ render() {
1113
+ return <div>{this.props.text}</div>;
1114
+ }
1115
+ componentDidMount() {
1116
+ mounts++;
1117
+ // This should be called only once but we guard just in case.
1118
+ if (!this.props.rendered) {
1119
+ this.props.onChange({rendered: true});
1120
+ }
1121
+ }
1122
+ }
1123
+
1124
+ var container = document.createElement('div');
1125
+ function render() {
1126
+ ReactDOM.render(
1127
+ <Editor
1128
+ onChange={(newProps) => {
1129
+ props = {...props, ...newProps};
1130
+ render();
1131
+ }}
1132
+ {...props}
1133
+ />,
1134
+ container
1135
+ );
1136
+ }
1137
+
1138
+ var props = {text: 'hello', rendered: false};
1139
+ render();
1140
+ props = {...props, text: 'goodbye'};
1141
+ render();
1142
+ expect(container.textContent).toBe('goodbye');
1143
+ expect(mounts).toBe(1);
1144
+ });
1145
});