@samitouri / QOS-React-2 / commits / c978f789cc

[Fiber] Push class context providers even if they crash (#8627)

* Push class context providers early Previously we used to push them only after the instance was available. This caused issues in cases an error is thrown during componentWillMount(). In that case we never got to pushing the provider in the begin phase, but in complete phase the provider check returned true since the instance existed by that point. As a result we got mismatching context pops. We solve the issue by making the context check independent of whether the instance actually exists. Instead we're checking the type itself. This lets us push class context early. However there's another problem: we might not know the context value. If the instance is not yet created, we can't call getChildContext on it. To fix this, we are introducing a way to replace current value on the stack, and a way to read the previous value. This also helps remove some branching and split the memoized from invalidated code paths. * Add a now-passing test from #8604 Also rename another test to have a shorter name. * Move isContextProvider() checks into push() and pop() All uses of push() and pop() are guarded by it anyway. This makes it more similar to how we use host context. There is only one other place where isContextProvider() is used and that's legacy code needed for renderSubtree(). * Clarify why we read the previous context * Use invariant instead of throwing an error * Fix off-by-one in ReactFiberStack * Manually keep track of the last parent context The previous algorithm was flawed and worked by accident, as shown by the failing tests after an off-by-one was fixed. The implementation of getPrevious() was incorrect because the context stack currently has no notion of a previous value per cursor. Instead, we are caching the previous value directly in the ReactFiberContext in a local variable. Additionally, we are using push() and pop() instead of adding a new replace() method.

Dan Abramov committed Dec 22, 2016 at 10:36 UTC c978f789cc096afc7a7d86087a872d8e1d5ffcd3
7 files changed +190 -47
scripts/fiber/tests-passing.txt
+4 -1
@@ -1157,7 +1157,8 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
1157 * provides context when reusing work
1158 * reads context when setState is below the provider
1159 * reads context when setState is above the provider
1160 -* maintains the correct context index when context proviers are bailed out due to low priority
1160 +* maintains the correct context when providers bail out due to low priority
1161 +* maintains the correct context when unwinding due to an error in render
1162
1163 src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js
1164 * catches render error in a boundary during full deferred mounting
@@ -1413,6 +1414,8 @@ src/renderers/shared/shared/__tests__/ReactErrorBoundaries-test.js
1414 * renders an error state if child throws in render
1415 * renders an error state if child throws in constructor
1416 * renders an error state if child throws in componentWillMount
1417 +* renders an error state if context provider throws in componentWillMount
1418 +* renders an error state if module-style context provider throws in componentWillMount
1419 * mounts the error message if mounting fails
1420 * propagates errors on retry on mounting
1421 * propagates errors inside boundary during componentWillMount
src/renderers/shared/fiber/ReactFiberBeginWork.js
+24 -15
@@ -31,10 +31,10 @@ var {
31 var ReactTypeOfWork = require('ReactTypeOfWork');
32 var {
33 getMaskedContext,
34 - isContextProvider,
34 hasContextChanged,
35 pushContextProvider,
36 pushTopLevelContextObject,
37 + invalidateContextProvider,
38 } = require('ReactFiberContext');
39 var {
40 IndeterminateComponent,
@@ -215,6 +215,11 @@ module.exports = function<T, P, I, TI, C, CX>(
215 }
216
217 function updateClassComponent(current : ?Fiber, workInProgress : Fiber, priorityLevel : PriorityLevel) {
218 + // Push context providers early to prevent context stack mismatches.
219 + // During mounting we don't know the child context yet as the instance doesn't exist.
220 + // We will invalidate the child context in finishClassComponent() right after rendering.
221 + const hasContext = pushContextProvider(workInProgress);
222 +
223 let shouldUpdate;
224 if (!current) {
225 if (!workInProgress.stateNode) {
@@ -229,10 +234,15 @@ module.exports = function<T, P, I, TI, C, CX>(
234 } else {
235 shouldUpdate = updateClassInstance(current, workInProgress, priorityLevel);
236 }
232 - return finishClassComponent(current, workInProgress, shouldUpdate);
237 + return finishClassComponent(current, workInProgress, shouldUpdate, hasContext);
238 }
239
235 - function finishClassComponent(current : ?Fiber, workInProgress : Fiber, shouldUpdate : boolean) {
240 + function finishClassComponent(
241 + current : ?Fiber,
242 + workInProgress : Fiber,
243 + shouldUpdate : boolean,
244 + hasContext : boolean,
245 + ) {
246 // Schedule side-effects
247 if (shouldUpdate) {
248 workInProgress.effectTag |= Update;
@@ -246,11 +256,6 @@ module.exports = function<T, P, I, TI, C, CX>(
256 workInProgress.effectTag |= Update;
257 }
258 }
249 -
250 - // Don't forget to push the context before returning.
251 - if (isContextProvider(workInProgress)) {
252 - pushContextProvider(workInProgress, false);
253 - }
259 return bailoutOnAlreadyFinishedWork(current, workInProgress);
260 }
261
@@ -259,9 +264,10 @@ module.exports = function<T, P, I, TI, C, CX>(
264 ReactCurrentOwner.current = workInProgress;
265 const nextChildren = instance.render();
266 reconcileChildren(current, workInProgress, nextChildren);
262 - // Put context on the stack because we will work on children
263 - if (isContextProvider(workInProgress)) {
264 - pushContextProvider(workInProgress, true);
267 +
268 + // The context might have changed so we need to recalculate it.
269 + if (hasContext) {
270 + invalidateContextProvider(workInProgress);
271 }
272 return workInProgress.child;
273 }
@@ -417,9 +423,14 @@ module.exports = function<T, P, I, TI, C, CX>(
423 if (typeof value === 'object' && value && typeof value.render === 'function') {
424 // Proceed under the assumption that this is a class instance
425 workInProgress.tag = ClassComponent;
426 +
427 + // Push context providers early to prevent context stack mismatches.
428 + // During mounting we don't know the child context yet as the instance doesn't exist.
429 + // We will invalidate the child context in finishClassComponent() right after rendering.
430 + const hasContext = pushContextProvider(workInProgress);
431 adoptClassInstance(workInProgress, value);
432 mountClassInstance(workInProgress, priorityLevel);
422 - return finishClassComponent(current, workInProgress, true);
433 + return finishClassComponent(current, workInProgress, true, hasContext);
434 } else {
435 // Proceed under the assumption that this is a functional component
436 workInProgress.tag = FunctionalComponent;
@@ -535,9 +546,7 @@ module.exports = function<T, P, I, TI, C, CX>(
546 // See PR 8590 discussion for context
547 switch (workInProgress.tag) {
548 case ClassComponent:
538 - if (isContextProvider(workInProgress)) {
539 - pushContextProvider(workInProgress, false);
540 - }
549 + pushContextProvider(workInProgress);
550 break;
551 case HostPortal:
552 pushHostContainer(workInProgress, workInProgress.stateNode.containerInfo);
src/renderers/shared/fiber/ReactFiberCompleteWork.js
+1 -4
@@ -21,7 +21,6 @@ import type { ReifiedYield } from 'ReactReifiedYield';
21
22 var { reconcileChildFibers } = require('ReactChildFiber');
23 var {
24 - isContextProvider,
24 popContextProvider,
25 } = require('ReactFiberContext');
26 var ReactTypeOfWork = require('ReactTypeOfWork');
@@ -175,9 +174,7 @@ module.exports = function<T, P, I, TI, C, CX>(
174 return null;
175 case ClassComponent: {
176 // We are leaving this subtree, so pop context if any.
178 - if (isContextProvider(workInProgress)) {
179 - popContextProvider(workInProgress);
180 - }
177 + popContextProvider(workInProgress);
178 // Don't use the state queue to compute the memoized state. We already
179 // merged it and assigned it to the instance. Transfer it from there.
180 // Also need to transfer the props, because pendingProps will be null
src/renderers/shared/fiber/ReactFiberContext.js
+60 -22
@@ -35,11 +35,25 @@ if (__DEV__) {
35 var checkReactTypeSpec = require('checkReactTypeSpec');
36 }
37
38 -let contextStackCursor : StackCursor<?Object> = createCursor((null: ?Object));
38 +// A cursor to the current merged context object on the stack.
39 +let contextStackCursor : StackCursor<Object> = createCursor(emptyObject);
40 +// A cursor to a boolean indicating whether the context has changed.
41 let didPerformWorkStackCursor : StackCursor<boolean> = createCursor(false);
40 -
41 -function getUnmaskedContext() {
42 - return contextStackCursor.current || emptyObject;
42 +// Keep track of the previous context object that was on the stack.
43 +// We use this to get access to the parent context after we have already
44 +// pushed the next context provider, and now need to merge their contexts.
45 +let previousContext : Object = emptyObject;
46 +
47 +function getUnmaskedContext(workInProgress : Fiber) : Object {
48 + const hasOwnContext = isContextProvider(workInProgress);
49 + if (hasOwnContext) {
50 + // If the fiber is a context provider itself, when we read its context
51 + // we have already pushed its own child context on the stack. A context
52 + // provider should not "see" its own child context. Therefore we read the
53 + // previous (parent) context instead for a context provider.
54 + return previousContext;
55 + }
56 + return contextStackCursor.current;
57 }
58
59 exports.getMaskedContext = function(workInProgress : Fiber) {
@@ -49,9 +63,8 @@ exports.getMaskedContext = function(workInProgress : Fiber) {
63 return emptyObject;
64 }
65
52 - const unmaskedContext = getUnmaskedContext();
66 + const unmaskedContext = getUnmaskedContext(workInProgress);
67 const context = {};
54 -
68 for (let key in contextTypes) {
69 context[key] = unmaskedContext[key];
70 }
@@ -71,14 +84,16 @@ exports.hasContextChanged = function() : boolean {
84 function isContextProvider(fiber : Fiber) : boolean {
85 return (
86 fiber.tag === ClassComponent &&
74 - // Instance might be null, if the fiber errored during construction
75 - fiber.stateNode &&
76 - typeof fiber.stateNode.getChildContext === 'function'
87 + fiber.type.childContextTypes != null
88 );
89 }
90 exports.isContextProvider = isContextProvider;
91
92 function popContextProvider(fiber : Fiber) : void {
93 + if (!isContextProvider(fiber)) {
94 + return;
95 + }
96 +
97 pop(didPerformWorkStackCursor, fiber);
98 pop(contextStackCursor, fiber);
99 }
@@ -117,25 +132,48 @@ function processChildContext(fiber : Fiber, parentContext : Object, isReconcilin
132 }
133 exports.processChildContext = processChildContext;
134
120 -exports.pushContextProvider = function(workInProgress : Fiber, didPerformWork : boolean) : void {
121 - const instance = workInProgress.stateNode;
122 - const memoizedMergedChildContext = instance.__reactInternalMemoizedMergedChildContext;
123 - const canReuseMergedChildContext = !didPerformWork && memoizedMergedChildContext != null;
124 -
125 - let mergedContext = null;
126 - if (canReuseMergedChildContext) {
127 - mergedContext = memoizedMergedChildContext;
128 - } else {
129 - mergedContext = processChildContext(workInProgress, getUnmaskedContext(), true);
130 - instance.__reactInternalMemoizedMergedChildContext = mergedContext;
135 +exports.pushContextProvider = function(workInProgress : Fiber) : boolean {
136 + if (!isContextProvider(workInProgress)) {
137 + return false;
138 }
139
140 + const instance = workInProgress.stateNode;
141 + // We push the context as early as possible to ensure stack integrity.
142 + // If the instance does not exist yet, we will push null at first,
143 + // and replace it on the stack later when invalidating the context.
144 + const memoizedMergedChildContext = (
145 + instance &&
146 + instance.__reactInternalMemoizedMergedChildContext
147 + ) || emptyObject;
148 +
149 + // Remember the parent context so we can merge with it later.
150 + previousContext = contextStackCursor.current;
151 + push(contextStackCursor, memoizedMergedChildContext, workInProgress);
152 + push(didPerformWorkStackCursor, false, workInProgress);
153 +
154 + return true;
155 +};
156 +
157 +exports.invalidateContextProvider = function(workInProgress : Fiber) : void {
158 + const instance = workInProgress.stateNode;
159 + invariant(instance, 'Expected to have an instance by this point.');
160 +
161 + // Merge parent and own context.
162 + const mergedContext = processChildContext(workInProgress, previousContext, true);
163 + instance.__reactInternalMemoizedMergedChildContext = mergedContext;
164 +
165 + // Replace the old (or empty) context with the new one.
166 + // It is important to unwind the context in the reverse order.
167 + pop(didPerformWorkStackCursor, workInProgress);
168 + pop(contextStackCursor, workInProgress);
169 + // Now push the new context and mark that it has changed.
170 push(contextStackCursor, mergedContext, workInProgress);
134 - push(didPerformWorkStackCursor, didPerformWork, workInProgress);
171 + push(didPerformWorkStackCursor, true, workInProgress);
172 };
173
174 exports.resetContext = function() : void {
138 - contextStackCursor.current = null;
175 + previousContext = emptyObject;
176 + contextStackCursor.current = emptyObject;
177 didPerformWorkStackCursor.current = false;
178 };
179
src/renderers/shared/fiber/ReactFiberScheduler.js
+1 -4
@@ -18,7 +18,6 @@ import type { HostConfig, Deadline } from 'ReactFiberReconciler';
18 import type { PriorityLevel } from 'ReactPriorityLevel';
19
20 var {
21 - isContextProvider,
21 popContextProvider,
22 } = require('ReactFiberContext');
23 const { reset } = require('ReactFiberStack');
@@ -957,9 +956,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C
956 while (node && (node !== to) && (node.alternate !== to)) {
957 switch (node.tag) {
958 case ClassComponent:
960 - if (isContextProvider(node)) {
961 - popContextProvider(node);
962 - }
959 + popContextProvider(node);
960 break;
961 case HostComponent:
962 popHostContext(node);
src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
+48 -1
@@ -1932,7 +1932,7 @@ describe('ReactIncremental', () => {
1932 ]);
1933 });
1934
1935 - it('maintains the correct context index when context proviers are bailed out due to low priority', () => {
1935 + it('maintains the correct context when providers bail out due to low priority', () => {
1936 class Root extends React.Component {
1937 render() {
1938 return <Middle {...this.props} />;
@@ -1974,4 +1974,51 @@ describe('ReactIncremental', () => {
1974 instance.setState({});
1975 ReactNoop.flush();
1976 });
1977 +
1978 + it('maintains the correct context when unwinding due to an error in render', () => {
1979 + class Root extends React.Component {
1980 + unstable_handleError(error) {
1981 + // If context is pushed/popped correctly,
1982 + // This method will be used to handle the intentionally-thrown Error.
1983 + }
1984 + render() {
1985 + return <ContextProvider depth={1} />;
1986 + }
1987 + }
1988 +
1989 + let instance;
1990 +
1991 + class ContextProvider extends React.Component {
1992 + constructor(props, context) {
1993 + super(props, context);
1994 + this.state = {};
1995 + if (props.depth === 1) {
1996 + instance = this;
1997 + }
1998 + }
1999 + static childContextTypes = {};
2000 + getChildContext() {
2001 + return {};
2002 + }
2003 + render() {
2004 + if (this.state.throwError) {
2005 + throw Error();
2006 + }
2007 + return this.props.depth < 4
2008 + ? <ContextProvider depth={this.props.depth + 1} />
2009 + : <div />;
2010 + }
2011 + }
2012 +
2013 + // Init
2014 + ReactNoop.render(<Root />);
2015 + ReactNoop.flush();
2016 +
2017 + // Trigger an update in the middle of the tree
2018 + // This is necessary to reproduce the error as it curently exists.
2019 + instance.setState({
2020 + throwError: true,
2021 + });
2022 + ReactNoop.flush();
2023 + });
2024 });
src/renderers/shared/shared/__tests__/ReactErrorBoundaries-test.js
+52
@@ -733,6 +733,58 @@ describe('ReactErrorBoundaries', () => {
733 ]);
734 });
735
736 + it('renders an error state if context provider throws in componentWillMount', () => {
737 + class BrokenComponentWillMountWithContext extends React.Component {
738 + static childContextTypes = {foo: React.PropTypes.number};
739 + getChildContext() {
740 + return {foo: 42};
741 + }
742 + render() {
743 + return <div>{this.props.children}</div>;
744 + }
745 + componentWillMount() {
746 + throw new Error('Hello');
747 + }
748 + }
749 +
750 + var container = document.createElement('div');
751 + ReactDOM.render(
752 + <ErrorBoundary>
753 + <BrokenComponentWillMountWithContext />
754 + </ErrorBoundary>,
755 + container
756 + );
757 + expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
758 + });
759 +
760 + it('renders an error state if module-style context provider throws in componentWillMount', () => {
761 + function BrokenComponentWillMountWithContext() {
762 + return {
763 + getChildContext() {
764 + return {foo: 42};
765 + },
766 + render() {
767 + return <div>{this.props.children}</div>;
768 + },
769 + componentWillMount() {
770 + throw new Error('Hello');
771 + },
772 + };
773 + }
774 + BrokenComponentWillMountWithContext.childContextTypes = {
775 + foo: React.PropTypes.number,
776 + };
777 +
778 + var container = document.createElement('div');
779 + ReactDOM.render(
780 + <ErrorBoundary>
781 + <BrokenComponentWillMountWithContext />
782 + </ErrorBoundary>,
783 + container
784 + );
785 + expect(container.firstChild.textContent).toBe('Caught an error: Hello.');
786 + });
787 +
788 it('mounts the error message if mounting fails', () => {
789 function renderError(error) {
790 return (