From bc1ea9cd96c81467c574b134349eafc303835d0e Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 25 Jul 2018 14:22:27 -0700 Subject: [PATCH 01/25] Handle errors thrown in gDSFP of a module-style context provider (#13269) Context should be pushed before calling any user code, so if it errors the stack unwinds correctly. --- .../src/ReactFiberBeginWork.js | 9 ++++---- ...tIncrementalErrorHandling-test.internal.js | 22 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 2f64fb614667..dcd402072a28 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -618,6 +618,11 @@ function mountIndeterminateComponent( // Proceed under the assumption that this is a class instance workInProgress.tag = ClassComponent; + // Push context providers early to prevent context stack mismatches. + // During mounting we don't know the child context yet as the instance doesn't exist. + // We will invalidate the child context in finishClassComponent() right after rendering. + const hasContext = pushLegacyContextProvider(workInProgress); + workInProgress.memoizedState = value.state !== null && value.state !== undefined ? value.state : null; @@ -630,10 +635,6 @@ function mountIndeterminateComponent( ); } - // Push context providers early to prevent context stack mismatches. - // During mounting we don't know the child context yet as the instance doesn't exist. - // We will invalidate the child context in finishClassComponent() right after rendering. - const hasContext = pushLegacyContextProvider(workInProgress); adoptClassInstance(workInProgress, value); mountClassInstance(workInProgress, renderExpirationTime); return finishClassComponent( diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index fd5b1c8a33fc..0b2b6551aa73 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -1469,4 +1469,26 @@ describe('ReactIncrementalErrorHandling', () => { ReactNoop.flushDeferredPri(); expect(ReactNoop.getChildren()).toEqual([span('Caught an error: Hello')]); }); + + it('handles error thrown inside getDerivedStateFromProps of a module-style context provider', () => { + function Provider() { + return { + getChildContext() { + return {foo: 'bar'}; + }, + render() { + return 'Hi'; + }, + }; + } + Provider.childContextTypes = { + x: () => {}, + }; + Provider.getDerivedStateFromProps = () => { + throw new Error('Oops!'); + }; + + ReactNoop.render(); + expect(() => ReactNoop.flush()).toThrow('Oops!'); + }); }); From 840cb1a2683dd7c0f3beafd094081d16b636721e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 27 Jul 2018 16:50:20 +0200 Subject: [PATCH 02/25] Add an invariant to createRoot() to validate containers (#13279) --- packages/react-dom/src/__tests__/ReactDOMRoot-test.js | 8 ++++++++ packages/react-dom/src/client/ReactDOM.js | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMRoot-test.js b/packages/react-dom/src/__tests__/ReactDOMRoot-test.js index deded7213005..1d3bc4074879 100644 --- a/packages/react-dom/src/__tests__/ReactDOMRoot-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMRoot-test.js @@ -366,4 +366,12 @@ describe('ReactDOMRoot', () => { ); expect(() => batch.commit()).toThrow('Element type is invalid'); }); + + it('throws a good message on invalid containers', () => { + expect(() => { + ReactDOM.unstable_createRoot(
Hi
); + }).toThrow( + 'unstable_createRoot(...): Target container is not a DOM element.', + ); + }); }); diff --git a/packages/react-dom/src/client/ReactDOM.js b/packages/react-dom/src/client/ReactDOM.js index 55760eaff53d..8a179d1e55a4 100644 --- a/packages/react-dom/src/client/ReactDOM.js +++ b/packages/react-dom/src/client/ReactDOM.js @@ -758,6 +758,10 @@ ReactDOM.unstable_createRoot = function createRoot( container: DOMContainer, options?: RootOptions, ): ReactRoot { + invariant( + isValidContainer(container), + 'unstable_createRoot(...): Target container is not a DOM element.', + ); const hydrate = options != null && options.hydrate === true; return new ReactRoot(container, true, hydrate); }; From 2a2ef7e0fd86612404676b15bb3dd9df7ade536e Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 27 Jul 2018 13:42:17 -0700 Subject: [PATCH 03/25] Remove unnecessary branching from updateContextProvider (#13282) This code had gotten unnecessarily complex after some recent changes. Cleaned it up a bit. --- .../src/ReactFiberBeginWork.js | 84 ++++--------------- .../src/ReactFiberNewContext.js | 41 ++++++++- packages/react/src/ReactContext.js | 3 +- packages/shared/ReactTypes.js | 1 - 4 files changed, 54 insertions(+), 75 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index dcd402072a28..fb71d1858196 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -73,6 +73,7 @@ import { propagateContextChange, readContext, prepareToReadContext, + calculateChangedBits, } from './ReactFiberNewContext'; import { markActualRenderTimeStarted, @@ -824,88 +825,34 @@ function updateContextProvider(current, workInProgress, renderExpirationTime) { // Initial render changedBits = MAX_SIGNED_31_BIT_INT; } else { - if (oldProps.value === newProps.value) { + const oldValue = oldProps.value; + changedBits = calculateChangedBits(context, newValue, oldValue); + if (changedBits === 0) { // No change. Bailout early if children are the same. if ( oldProps.children === newProps.children && !hasLegacyContextChanged() ) { - workInProgress.stateNode = 0; - pushProvider(workInProgress); + pushProvider(workInProgress, 0); return bailoutOnAlreadyFinishedWork( current, workInProgress, renderExpirationTime, ); } - changedBits = 0; } else { - const oldValue = oldProps.value; - // Use Object.is to compare the new context value to the old value. - // Inlined Object.is polyfill. - // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is - if ( - (oldValue === newValue && - (oldValue !== 0 || 1 / oldValue === 1 / newValue)) || - (oldValue !== oldValue && newValue !== newValue) // eslint-disable-line no-self-compare - ) { - // No change. Bailout early if children are the same. - if ( - oldProps.children === newProps.children && - !hasLegacyContextChanged() - ) { - workInProgress.stateNode = 0; - pushProvider(workInProgress); - return bailoutOnAlreadyFinishedWork( - current, - workInProgress, - renderExpirationTime, - ); - } - changedBits = 0; - } else { - changedBits = - typeof context._calculateChangedBits === 'function' - ? context._calculateChangedBits(oldValue, newValue) - : MAX_SIGNED_31_BIT_INT; - if (__DEV__) { - warning( - (changedBits & MAX_SIGNED_31_BIT_INT) === changedBits, - 'calculateChangedBits: Expected the return value to be a ' + - '31-bit integer. Instead received: %s', - changedBits, - ); - } - changedBits |= 0; - - if (changedBits === 0) { - // No change. Bailout early if children are the same. - if ( - oldProps.children === newProps.children && - !hasLegacyContextChanged() - ) { - workInProgress.stateNode = 0; - pushProvider(workInProgress); - return bailoutOnAlreadyFinishedWork( - current, - workInProgress, - renderExpirationTime, - ); - } - } else { - propagateContextChange( - workInProgress, - context, - changedBits, - renderExpirationTime, - ); - } - } + // The context value changed. Search for matching consumers and schedule + // them to update. + propagateContextChange( + workInProgress, + context, + changedBits, + renderExpirationTime, + ); } } - workInProgress.stateNode = changedBits; - pushProvider(workInProgress); + pushProvider(workInProgress, changedBits); const newChildren = newProps.children; reconcileChildren(current, workInProgress, newChildren, renderExpirationTime); @@ -1049,8 +996,7 @@ function beginWork( ); break; case ContextProvider: - workInProgress.stateNode = 0; - pushProvider(workInProgress); + pushProvider(workInProgress, 0); break; case Profiler: if (enableProfilerTimer) { diff --git a/packages/react-reconciler/src/ReactFiberNewContext.js b/packages/react-reconciler/src/ReactFiberNewContext.js index 91d0b52826ed..e8f8e1b1e22e 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.js @@ -26,7 +26,9 @@ import {NoWork} from './ReactFiberExpirationTime'; import {ContextProvider} from 'shared/ReactTypeOfWork'; import invariant from 'shared/invariant'; +import warning from 'shared/warning'; +import MAX_SIGNED_31_BIT_INT from './maxSigned31BitInt'; const valueCursor: StackCursor = createCursor(null); const changedBitsCursor: StackCursor = createCursor(0); @@ -48,7 +50,7 @@ export function resetContextDependences(): void { lastContextWithAllBitsObserved = null; } -export function pushProvider(providerFiber: Fiber): void { +export function pushProvider(providerFiber: Fiber, changedBits: number): void { const context: ReactContext = providerFiber.type._context; if (isPrimaryRenderer) { @@ -56,7 +58,7 @@ export function pushProvider(providerFiber: Fiber): void { push(valueCursor, context._currentValue, providerFiber); context._currentValue = providerFiber.pendingProps.value; - context._changedBits = providerFiber.stateNode; + context._changedBits = changedBits; if (__DEV__) { warningWithoutStack( context._currentRenderer === undefined || @@ -72,7 +74,7 @@ export function pushProvider(providerFiber: Fiber): void { push(valueCursor, context._currentValue2, providerFiber); context._currentValue2 = providerFiber.pendingProps.value; - context._changedBits2 = providerFiber.stateNode; + context._changedBits2 = changedBits; if (__DEV__) { warningWithoutStack( context._currentRenderer2 === undefined || @@ -103,6 +105,39 @@ export function popProvider(providerFiber: Fiber): void { } } +export function calculateChangedBits( + context: ReactContext, + newValue: T, + oldValue: T, +) { + // Use Object.is to compare the new context value to the old value. Inlined + // Object.is polyfill. + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is + if ( + (oldValue === newValue && + (oldValue !== 0 || 1 / oldValue === 1 / (newValue: any))) || + (oldValue !== oldValue && newValue !== newValue) // eslint-disable-line no-self-compare + ) { + // No change + return 0; + } else { + const changedBits = + typeof context._calculateChangedBits === 'function' + ? context._calculateChangedBits(oldValue, newValue) + : MAX_SIGNED_31_BIT_INT; + + if (__DEV__) { + warning( + (changedBits & MAX_SIGNED_31_BIT_INT) === changedBits, + 'calculateChangedBits: Expected the return value to be a ' + + '31-bit integer. Instead received: %s', + changedBits, + ); + } + return changedBits | 0; + } +} + export function propagateContextChange( workInProgress: Fiber, context: ReactContext, diff --git a/packages/react/src/ReactContext.js b/packages/react/src/ReactContext.js index 02ea7959af53..bdbb55f9389f 100644 --- a/packages/react/src/ReactContext.js +++ b/packages/react/src/ReactContext.js @@ -50,13 +50,12 @@ export function createContext( const context: ReactContext = { $$typeof: REACT_CONTEXT_TYPE, _calculateChangedBits: calculateChangedBits, - _defaultValue: defaultValue, - _currentValue: defaultValue, // As a workaround to support multiple concurrent renderers, we categorize // some renderers as primary and others as secondary. We only expect // there to be two concurrent renderers at most: React Native (primary) and // Fabric (secondary); React DOM (primary) and React ART (secondary). // Secondary renderers store their context values on separate fields. + _currentValue: defaultValue, _currentValue2: defaultValue, _changedBits: 0, _changedBits2: 0, diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index bff9413c8285..c662a285aa01 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -82,7 +82,6 @@ export type ReactContext = { unstable_read: () => T, _calculateChangedBits: ((a: T, b: T) => number) | null, - _defaultValue: T, _currentValue: T, _currentValue2: T, From 0182a74632b66a226921ca13dcfe359c67e15c6f Mon Sep 17 00:00:00 2001 From: Konstantin Yakushin Date: Wed, 1 Aug 2018 18:16:34 +0300 Subject: [PATCH 04/25] Fix a crash when using dynamic children in