From 12bee76ff7f8508a515271e2ec0cefe2065547ec Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 11 Nov 2016 21:14:20 +0000 Subject: [PATCH 01/14] Implement basic support for context --- .../__tests__/ReactContextValidator-test.js | 45 ++++- .../shared/fiber/ReactFiberBeginWork.js | 25 ++- .../shared/fiber/ReactFiberClassComponent.js | 57 ++++--- .../shared/fiber/ReactFiberCompleteWork.js | 38 +++-- .../shared/fiber/ReactFiberContext.js | 91 ++++++++++ .../shared/fiber/ReactFiberTreeReflection.js | 11 ++ .../fiber/__tests__/ReactIncremental-test.js | 158 ++++++++++++++++++ 7 files changed, 380 insertions(+), 45 deletions(-) create mode 100644 src/renderers/shared/fiber/ReactFiberContext.js diff --git a/src/isomorphic/classic/__tests__/ReactContextValidator-test.js b/src/isomorphic/classic/__tests__/ReactContextValidator-test.js index 6e237e65379..844a9e80269 100644 --- a/src/isomorphic/classic/__tests__/ReactContextValidator-test.js +++ b/src/isomorphic/classic/__tests__/ReactContextValidator-test.js @@ -70,11 +70,10 @@ describe('ReactContextValidator', () => { expect(instance.refs.child.context).toEqual({foo: 'abc'}); }); - it('should filter context properly in callbacks', () => { + it('should pass next context to lifecycles', () => { var actualComponentWillReceiveProps; var actualShouldComponentUpdate; var actualComponentWillUpdate; - var actualComponentDidUpdate; var Parent = React.createClass({ childContextTypes: { @@ -113,6 +112,45 @@ describe('ReactContextValidator', () => { actualComponentWillUpdate = nextContext; }, + render: function() { + return
; + }, + }); + + var container = document.createElement('div'); + ReactDOM.render(, container); + ReactDOM.render(, container); + expect(actualComponentWillReceiveProps).toEqual({foo: 'def'}); + expect(actualShouldComponentUpdate).toEqual({foo: 'def'}); + expect(actualComponentWillUpdate).toEqual({foo: 'def'}); + }); + + it('should pass previous context to lifecycles', () => { + var actualComponentDidUpdate; + + var Parent = React.createClass({ + childContextTypes: { + foo: React.PropTypes.string.isRequired, + bar: React.PropTypes.string.isRequired, + }, + + getChildContext: function() { + return { + foo: this.props.foo, + bar: 'bar', + }; + }, + + render: function() { + return ; + }, + }); + + var Component = React.createClass({ + contextTypes: { + foo: React.PropTypes.string, + }, + componentDidUpdate: function(prevProps, prevState, prevContext) { actualComponentDidUpdate = prevContext; }, @@ -125,9 +163,6 @@ describe('ReactContextValidator', () => { var container = document.createElement('div'); ReactDOM.render(, container); ReactDOM.render(, container); - expect(actualComponentWillReceiveProps).toEqual({foo: 'def'}); - expect(actualShouldComponentUpdate).toEqual({foo: 'def'}); - expect(actualComponentWillUpdate).toEqual({foo: 'def'}); expect(actualComponentDidUpdate).toEqual({foo: 'abc'}); }); diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index b35567478c7..55f7128f524 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -23,7 +23,13 @@ var { reconcileChildFibersInPlace, cloneChildFibers, } = require('ReactChildFiber'); + var ReactTypeOfWork = require('ReactTypeOfWork'); +var { + getMaskedContext, + pushContextProvider, + resetContext, +} = require('ReactFiberContext'); var { IndeterminateComponent, FunctionalComponent, @@ -144,6 +150,7 @@ module.exports = function( function updateFunctionalComponent(current, workInProgress) { var fn = workInProgress.type; var props = workInProgress.pendingProps; + var context = getMaskedContext(workInProgress); // TODO: Disable this before release, since it is not part of the public API // I use this for testing to compare the relative overhead of classes. @@ -159,9 +166,9 @@ module.exports = function( if (__DEV__) { ReactCurrentOwner.current = workInProgress; - nextChildren = fn(props); + nextChildren = fn(props, context); } else { - nextChildren = fn(props); + nextChildren = fn(props, context); } reconcileChildren(current, workInProgress, nextChildren); return workInProgress.child; @@ -182,11 +189,14 @@ module.exports = function( } else { shouldUpdate = updateClassInstance(current, workInProgress); } + const instance = workInProgress.stateNode; + if (typeof instance.getChildContext === 'function') { + pushContextProvider(workInProgress); + } if (!shouldUpdate) { return bailoutOnAlreadyFinishedWork(current, workInProgress); } // Rerender - const instance = workInProgress.stateNode; ReactCurrentOwner.current = workInProgress; const nextChildren = instance.render(); reconcileChildren(current, workInProgress, nextChildren); @@ -249,13 +259,15 @@ module.exports = function( } var fn = workInProgress.type; var props = workInProgress.pendingProps; + var context = getMaskedContext(workInProgress); + var value; if (__DEV__) { ReactCurrentOwner.current = workInProgress; - value = fn(props); + value = fn(props, context); } else { - value = fn(props); + value = fn(props, context); } if (typeof value === 'object' && value && typeof value.render === 'function') { @@ -355,6 +367,9 @@ module.exports = function( } function beginWork(current : ?Fiber, workInProgress : Fiber, priorityLevel : PriorityLevel) : ?Fiber { + if (!workInProgress.return) { + resetContext(); + } if (workInProgress.pendingWorkPriority === NoWork || workInProgress.pendingWorkPriority > priorityLevel) { return bailoutOnLowPriority(current, workInProgress); diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index fb8a0352fa2..aa600db5c3b 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -15,19 +15,21 @@ import type { Fiber } from 'ReactFiber'; import type { UpdateQueue } from 'ReactFiberUpdateQueue'; +var { + getMaskedContext, +} = require('ReactFiberContext'); var { createUpdateQueue, addToQueue, addCallbackToQueue, mergeUpdateQueue, } = require('ReactFiberUpdateQueue'); -var { isMounted } = require('ReactFiberTreeReflection'); +var { getComponentName, isMounted } = require('ReactFiberTreeReflection'); var ReactInstanceMap = require('ReactInstanceMap'); var shallowEqual = require('shallowEqual'); var warning = require('warning'); var invariant = require('invariant'); - const isArray = Array.isArray; module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { @@ -74,7 +76,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { }, }; - function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState) { + function checkShouldComponentUpdate(workInProgress, oldProps, newProps, newState, newContext) { const updateQueue = workInProgress.updateQueue; if (oldProps === null || (updateQueue && updateQueue.isForced)) { return true; @@ -82,14 +84,14 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { const instance = workInProgress.stateNode; if (typeof instance.shouldComponentUpdate === 'function') { - const shouldUpdate = instance.shouldComponentUpdate(newProps, newState); + const shouldUpdate = instance.shouldComponentUpdate(newProps, newState, newContext); if (__DEV__) { warning( shouldUpdate !== undefined, '%s.shouldComponentUpdate(): Returned undefined instead of a ' + 'boolean value. Make sure to return true or false.', - getName(workInProgress, instance) + getComponentName(workInProgress) ); } @@ -107,19 +109,9 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { return true; } - function getName(workInProgress: Fiber, inst: any): string { - const type = workInProgress.type; - const constructor = inst && inst.constructor; - return ( - type.displayName || (constructor && constructor.displayName) || - type.name || (constructor && constructor.name) || - 'A Component' - ); - } - function checkClassInstance(workInProgress: Fiber, inst: any) { if (__DEV__) { - const name = getName(workInProgress, inst); + const name = getComponentName(workInProgress); const renderPresent = inst.render; warning( renderPresent, @@ -194,7 +186,15 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { invariant( false, '%s.state: must be set to an object or null', - getName(workInProgress, inst) + getComponentName(workInProgress) + ); + } + if (typeof inst.getChildContext === 'function') { + invariant( + typeof workInProgress.type.childContextTypes === 'object', + '%s.getChildContext(): childContextTypes must be defined in order to ' + + 'use getChildContext().', + getComponentName(workInProgress) ); } } @@ -209,7 +209,8 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { function constructClassInstance(workInProgress : Fiber) : any { const ctor = workInProgress.type; const props = workInProgress.pendingProps; - const instance = new ctor(props); + const context = getMaskedContext(workInProgress); + const instance = new ctor(props, context); checkClassInstance(workInProgress, instance); adoptClassInstance(workInProgress, instance); return instance; @@ -218,7 +219,6 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { // Invokes the mount life-cycles on a previously never rendered instance. function mountClassInstance(workInProgress : Fiber) : void { const instance = workInProgress.stateNode; - const state = instance.state || null; let props = workInProgress.pendingProps; @@ -228,6 +228,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { instance.props = props; instance.state = state; + instance.context = getMaskedContext(workInProgress); if (typeof instance.componentWillMount === 'function') { instance.componentWillMount(); @@ -253,6 +254,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { throw new Error('There should always be pending or memoized props.'); } } + const newContext = getMaskedContext(workInProgress); // TODO: Should we deal with a setState that happened after the last // componentWillMount and before this componentWillMount? Probably @@ -262,7 +264,8 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { workInProgress, workInProgress.memoizedProps, newProps, - newState + newState, + newContext )) { return false; } @@ -272,6 +275,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { const newInstance = constructClassInstance(workInProgress); newInstance.props = newProps; newInstance.state = newState = newInstance.state || null; + newInstance.context = getMaskedContext(workInProgress); if (typeof newInstance.componentWillMount === 'function') { newInstance.componentWillMount(); @@ -300,14 +304,16 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { throw new Error('There should always be pending or memoized props.'); } } + const oldContext = instance.context; + const newContext = getMaskedContext(workInProgress); // Note: During these life-cycles, instance.props/instance.state are what // ever the previously attempted to render - not the "current". However, // during componentDidUpdate we pass the "current" props. - if (oldProps !== newProps) { + if (oldProps !== newProps || oldContext !== newContext) { if (typeof instance.componentWillReceiveProps === 'function') { - instance.componentWillReceiveProps(newProps); + instance.componentWillReceiveProps(newProps, newContext); } } @@ -328,6 +334,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { if (oldProps === newProps && previousState === newState && + oldContext === newContext && updateQueue && !updateQueue.isForced) { return false; } @@ -336,18 +343,20 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { workInProgress, oldProps, newProps, - newState + newState, + newContext )) { // TODO: Should this get the new props/state updated regardless? return false; } if (typeof instance.componentWillUpdate === 'function') { - instance.componentWillUpdate(newProps, newState); + instance.componentWillUpdate(newProps, newState, newContext); } instance.props = newProps; instance.state = newState; + instance.context = newContext; return true; } diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index b0eea7c87e2..1444581805e 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -18,6 +18,7 @@ import type { HostConfig } from 'ReactFiberReconciler'; import type { ReifiedYield } from 'ReactReifiedYield'; var { reconcileChildFibers } = require('ReactChildFiber'); +var { popContextProvider } = require('ReactFiberContext'); var ReactTypeOfWork = require('ReactTypeOfWork'); var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect'); var { @@ -120,10 +121,11 @@ module.exports = function(config : HostConfig) { function completeWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { switch (workInProgress.tag) { - case FunctionalComponent: + case FunctionalComponent: { transferOutput(workInProgress.child, workInProgress); return null; - case ClassComponent: + } + case ClassComponent: { transferOutput(workInProgress.child, workInProgress); // Don't use the state queue to compute the memoized state. We already // merged it and assigned it to the instance. Transfer it from there. @@ -148,8 +150,13 @@ module.exports = function(config : HostConfig) { workInProgress.callbackList = updateQueue; markCallback(workInProgress); } + const instance = workInProgress.stateNode; + if (typeof instance.getChildContext === 'function') { + popContextProvider(); + } return null; - case HostContainer: + } + case HostContainer: { transferOutput(workInProgress.child, workInProgress); // We don't know if a container has updated any children so we always // need to update it right now. We schedule this side-effect before @@ -158,7 +165,8 @@ module.exports = function(config : HostConfig) { // are invoked. markUpdate(workInProgress); return null; - case HostComponent: + } + case HostComponent: { let newProps = workInProgress.pendingProps; if (current && workInProgress.stateNode != null) { // If we have an alternate, that means this is an update and we need to @@ -200,7 +208,8 @@ module.exports = function(config : HostConfig) { } workInProgress.memoizedProps = newProps; return null; - case HostText: + } + case HostText: { let newText = workInProgress.pendingProps; if (current && workInProgress.stateNode != null) { // If we have an alternate, that means this is an update and we need to @@ -222,25 +231,32 @@ module.exports = function(config : HostConfig) { } workInProgress.memoizedProps = newText; return null; - case CoroutineComponent: + } + case CoroutineComponent: { return moveCoroutineToHandlerPhase(current, workInProgress); - case CoroutineHandlerPhase: + } + case CoroutineHandlerPhase: { transferOutput(workInProgress.stateNode, workInProgress); // Reset the tag to now be a first phase coroutine. workInProgress.tag = CoroutineComponent; return null; - case YieldComponent: + } + case YieldComponent: { // Does nothing. return null; - case Fragment: + } + case Fragment: { transferOutput(workInProgress.child, workInProgress); return null; + } // Error cases - case IndeterminateComponent: + case IndeterminateComponent: { throw new Error('An indeterminate component should have become determinate before completing.'); - default: + } + default: { throw new Error('Unknown unit of work tag'); + } } } diff --git a/src/renderers/shared/fiber/ReactFiberContext.js b/src/renderers/shared/fiber/ReactFiberContext.js new file mode 100644 index 00000000000..4aa9310e2b7 --- /dev/null +++ b/src/renderers/shared/fiber/ReactFiberContext.js @@ -0,0 +1,91 @@ +/** + * Copyright 2013-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @providesModule ReactFiberContext + * @flow + */ + +'use strict'; + +import type { Fiber } from 'ReactFiber'; + +var emptyObject = require('emptyObject'); +var invariant = require('invariant'); +var { + getComponentName, +} = require('ReactFiberTreeReflection'); + +if (__DEV__) { + var checkReactTypeSpec = require('checkReactTypeSpec'); +} + +let index = -1; +const stack = []; + +function getUnmaskedContext() { + if (index === -1) { + return emptyObject; + } + return stack[index]; +} + +exports.getMaskedContext = function(fiber : Fiber) { + const type = fiber.type; + const contextTypes = type.contextTypes; + if (!contextTypes) { + return null; + } + + const unmaskedContext = getUnmaskedContext(); + const context = {}; + for (let key in contextTypes) { + context[key] = unmaskedContext[key]; + } + + if (__DEV__) { + const name = getComponentName(fiber); + const debugID = 0; // TODO: pass a real ID + checkReactTypeSpec(contextTypes, context, 'context', name, null, debugID); + } + + return context; +}; + +exports.popContextProvider = function() { + stack[index] = emptyObject; + index--; +}; + +exports.pushContextProvider = function(fiber : Fiber) { + const instance = fiber.stateNode; + const childContextTypes = fiber.type.childContextTypes; + const childContext = instance.getChildContext(); + + for (let contextKey in childContext) { + invariant( + contextKey in childContextTypes, + '%s.getChildContext(): key "%s" is not defined in childContextTypes.', + getComponentName(fiber), + contextKey + ); + } + if (__DEV__) { + const name = getComponentName(fiber); + const debugID = 0; // TODO: pass a real ID + checkReactTypeSpec(childContextTypes, childContext, 'childContext', name, null, debugID); + } + + const mergedContext = Object.assign({}, getUnmaskedContext(), childContext); + index++; + stack[index] = mergedContext; +}; + +exports.resetContext = function() { + index = -1; +}; + diff --git a/src/renderers/shared/fiber/ReactFiberTreeReflection.js b/src/renderers/shared/fiber/ReactFiberTreeReflection.js index d806992e74d..73dd021fb0d 100644 --- a/src/renderers/shared/fiber/ReactFiberTreeReflection.js +++ b/src/renderers/shared/fiber/ReactFiberTreeReflection.js @@ -114,3 +114,14 @@ exports.findCurrentHostFiber = function(component : ReactComponent { ]); expect(instance.state.n).toEqual(3); }); + + it('merges and masks context', () => { + var ops = []; + + class Intl extends React.Component { + static childContextTypes = { + locale: React.PropTypes.string, + }; + getChildContext() { + return { + locale: this.props.locale, + }; + } + render() { + ops.push('Intl ' + JSON.stringify(this.context)); + return this.props.children; + } + } + + class Router extends React.Component { + static childContextTypes = { + route: React.PropTypes.string, + }; + getChildContext() { + return { + route: this.props.route, + }; + } + render() { + ops.push('Router ' + JSON.stringify(this.context)); + return this.props.children; + } + } + + class ShowLocale extends React.Component { + static contextTypes = { + locale: React.PropTypes.string, + }; + + render() { + ops.push('ShowLocale ' + JSON.stringify(this.context)); + return this.context.locale; + } + } + + class ShowRoute extends React.Component { + static contextTypes = { + route: React.PropTypes.string, + }; + + render() { + ops.push('ShowRoute ' + JSON.stringify(this.context)); + return this.context.route; + } + } + + function ShowBoth(props, context) { + ops.push('ShowBoth ' + JSON.stringify(context)); + return `${context.route} in ${context.locale}`; + } + ShowBoth.contextTypes = { + locale: React.PropTypes.string, + route: React.PropTypes.string, + }; + + class ShowNeither extends React.Component { + render() { + ops.push('ShowNeither ' + JSON.stringify(this.context)); + return null; + } + } + + class Indirection extends React.Component { + render() { + ops.push('Indirection ' + JSON.stringify(this.context)); + return [ + , + , + , + + + , + , + ]; + } + } + + ops.length = []; + ReactNoop.render( + + +
+ +
+
+ ); + ReactNoop.flush(); + expect(ops).toEqual([ + 'Intl null', + 'ShowLocale {"locale":"fr"}', + 'ShowBoth {"locale":"fr"}', + ]); + + ops.length = []; + ReactNoop.render( + + +
+ +
+
+ ); + ReactNoop.flush(); + expect(ops).toEqual([ + 'Intl null', + 'ShowLocale {"locale":"de"}', + 'ShowBoth {"locale":"de"}', + ]); + + ops.length = []; + ReactNoop.render( + + +
+ +
+
+ ); + ReactNoop.flushDeferredPri(15); + expect(ops).toEqual([ + 'Intl null', + ]); + + ops.length = []; + ReactNoop.render( + + + + + + + + ); + ReactNoop.flush(); + expect(ops).toEqual([ + 'Intl null', + 'ShowLocale {"locale":"en"}', + 'Router null', + 'Indirection null', + 'ShowLocale {"locale":"en"}', + 'ShowRoute {"route":"/about"}', + 'ShowNeither null', + 'Intl null', + 'ShowBoth {"locale":"ru","route":"/about"}', + 'ShowBoth {"locale":"en","route":"/about"}', + 'ShowBoth {"locale":"en"}', + ]); + }); }); From a05d1abee8418c568ceabfdb846d5418c54edd20 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 11 Nov 2016 21:17:38 +0000 Subject: [PATCH 02/14] Minor style tweaks --- .../shared/fiber/ReactFiberClassComponent.js | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index aa600db5c3b..79a6a9ebc9e 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -109,10 +109,11 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { return true; } - function checkClassInstance(workInProgress: Fiber, inst: any) { + function checkClassInstance(workInProgress: Fiber) { + const instance = workInProgress.stateNode; if (__DEV__) { const name = getComponentName(workInProgress); - const renderPresent = inst.render; + const renderPresent = instance.render; warning( renderPresent, '%s(...): No `render` method found on the returned component ' + @@ -120,8 +121,8 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { name ); const noGetInitialStateOnES6 = ( - !inst.getInitialState || - inst.getInitialState.isReactClassApproved + !instance.getInitialState || + instance.getInitialState.isReactClassApproved ); warning( noGetInitialStateOnES6, @@ -131,8 +132,8 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { name ); const noGetDefaultPropsOnES6 = ( - !inst.getDefaultProps || - inst.getDefaultProps.isReactClassApproved + !instance.getDefaultProps || + instance.getDefaultProps.isReactClassApproved ); warning( noGetDefaultPropsOnES6, @@ -141,21 +142,21 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { 'Use a static property to define defaultProps instead.', name ); - const noInstancePropTypes = !inst.propTypes; + const noInstancePropTypes = !instance.propTypes; warning( noInstancePropTypes, 'propTypes was defined as an instance property on %s. Use a static ' + 'property to define propTypes instead.', name, ); - const noInstanceContextTypes = !inst.contextTypes; + const noInstanceContextTypes = !instance.contextTypes; warning( noInstanceContextTypes, 'contextTypes was defined as an instance property on %s. Use a static ' + 'property to define contextTypes instead.', name, ); - const noComponentShouldUpdate = typeof inst.componentShouldUpdate !== 'function'; + const noComponentShouldUpdate = typeof instance.componentShouldUpdate !== 'function'; warning( noComponentShouldUpdate, '%s has a method called ' + @@ -164,7 +165,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { 'expected to return a value.', name ); - const noComponentDidUnmount = typeof inst.componentDidUnmount !== 'function'; + const noComponentDidUnmount = typeof instance.componentDidUnmount !== 'function'; warning( noComponentDidUnmount, '%s has a method called ' + @@ -172,7 +173,7 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { 'Did you mean componentWillUnmount()?', name ); - const noComponentWillRecieveProps = typeof inst.componentWillRecieveProps !== 'function'; + const noComponentWillRecieveProps = typeof instance.componentWillRecieveProps !== 'function'; warning( noComponentWillRecieveProps, '%s has a method called ' + @@ -181,15 +182,15 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { ); } - const instanceState = inst.state; - if (instanceState && (typeof instanceState !== 'object' || isArray(instanceState))) { + const state = instance.state; + if (state && (typeof state !== 'object' || isArray(state))) { invariant( false, '%s.state: must be set to an object or null', getComponentName(workInProgress) ); } - if (typeof inst.getChildContext === 'function') { + if (typeof instance.getChildContext === 'function') { invariant( typeof workInProgress.type.childContextTypes === 'object', '%s.getChildContext(): childContextTypes must be defined in order to ' + @@ -211,8 +212,8 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { const props = workInProgress.pendingProps; const context = getMaskedContext(workInProgress); const instance = new ctor(props, context); - checkClassInstance(workInProgress, instance); adoptClassInstance(workInProgress, instance); + checkClassInstance(workInProgress); return instance; } @@ -319,21 +320,21 @@ module.exports = function(scheduleUpdate : (fiber: Fiber) => void) { // Compute the next state using the memoized state and the update queue. const updateQueue = workInProgress.updateQueue; - const previousState = workInProgress.memoizedState; + const oldState = workInProgress.memoizedState; // TODO: Previous state can be null. let newState; if (updateQueue) { if (!updateQueue.hasUpdate) { - newState = previousState; + newState = oldState; } else { - newState = mergeUpdateQueue(updateQueue, instance, previousState, newProps); + newState = mergeUpdateQueue(updateQueue, instance, oldState, newProps); } } else { - newState = previousState; + newState = oldState; } if (oldProps === newProps && - previousState === newState && + oldState === newState && oldContext === newContext && updateQueue && !updateQueue.isForced) { return false; From b8cca3711e940fdab848686e432d17f4c33d3241 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 11 Nov 2016 21:20:02 +0000 Subject: [PATCH 03/14] Update passing tests --- scripts/fiber/tests-failing.txt | 23 +--------------------- scripts/fiber/tests-passing-except-dev.txt | 3 --- scripts/fiber/tests-passing.txt | 18 +++++++++++++++++ 3 files changed, 19 insertions(+), 25 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 92ac48f24ed..3367df108b4 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -19,8 +19,7 @@ src/addons/transitions/__tests__/ReactCSSTransitionGroup-test.js * should transition from false to one src/isomorphic/classic/__tests__/ReactContextValidator-test.js -* should filter out context not in contextTypes -* should filter context properly in callbacks +* should pass previous context to lifecycles src/isomorphic/classic/class/__tests__/ReactBind-test.js * Holds reference to instance @@ -31,24 +30,9 @@ src/isomorphic/classic/class/__tests__/ReactBindOptout-test.js * works with mixins that have not opted out of autobinding * works with mixins that have opted out of autobinding -src/isomorphic/classic/class/__tests__/ReactClass-test.js -* renders based on context getInitialState - src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js * includes the owner name when passing null, undefined, boolean, or number -src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee -* renders based on context in the constructor -* supports this.context passed via getChildContext - -src/isomorphic/modern/class/__tests__/ReactES6Class-test.js -* renders based on context in the constructor -* supports this.context passed via getChildContext - -src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts -* renders based on context in the constructor -* supports this.context passed via getChildContext - src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js * should give context for PropType errors in nested components. @@ -441,11 +425,8 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js * should warn about `setState` on unmounted components * should warn about `setState` in render * should warn about `setState` in getChildContext -* should pass context to children when not owner * should pass context when re-rendered for static child * should pass context when re-rendered for static child within a composite component -* should pass context transitively -* should pass context when re-rendered * unmasked context propagates through updates * should trigger componentWillReceiveProps for context changes * should update refs if shouldComponentUpdate gives false @@ -467,10 +448,8 @@ src/renderers/shared/stack/reconciler/__tests__/ReactMultiChildText-test.js * should throw if rendering both HTML and children src/renderers/shared/stack/reconciler/__tests__/ReactStatelessComponent-test.js -* should pass context thru stateless component * should warn when stateless component returns array * should throw on string refs in pure functions -* should receive context * should warn when using non-React functions in JSX src/renderers/shared/stack/reconciler/__tests__/ReactUpdates-test.js diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt index 501853cc570..50d2a933219 100644 --- a/scripts/fiber/tests-passing-except-dev.txt +++ b/scripts/fiber/tests-passing-except-dev.txt @@ -12,9 +12,6 @@ src/isomorphic/classic/element/__tests__/ReactElementValidator-test.js * warns for keys with component stack info * should give context for PropType errors in nested components. -src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js -* should warn on invalid context types - src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js * should warn when using hyphenated style names * should warn when updating hyphenated style names diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index b5c82a50dbb..4069558127a 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -128,6 +128,10 @@ src/isomorphic/children/__tests__/sliceChildren-test.js * should allow static children to be sliced * should slice nested children +src/isomorphic/classic/__tests__/ReactContextValidator-test.js +* should filter out context not in contextTypes +* should pass next context to lifecycles + src/isomorphic/classic/class/__tests__/ReactBind-test.js * warns if you try to bind to this * does not warn if you pass an auto-bound method to setState @@ -149,6 +153,7 @@ src/isomorphic/classic/class/__tests__/ReactClass-test.js * should throw if a reserved property is in statics * should support statics * should work with object getInitialState() return values +* renders based on context getInitialState * should throw with non-object getInitialState() return values * should work with a null getInitialState() return value * should throw when using legacy factories @@ -354,6 +359,7 @@ src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee * renders a simple stateless component with prop * renders based on state using initial values in this.props * renders based on state using props in the constructor +* renders based on context in the constructor * renders only once when setting state in componentWillMount * should throw with non-object in the initial state property * should render with null in the initial state property @@ -365,6 +371,7 @@ src/isomorphic/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee * should warn when misspelling shouldComponentUpdate * should warn when misspelling componentWillReceiveProps * should throw AND warn when trying to access classic APIs +* supports this.context passed via getChildContext * supports classic refs * supports drilling through to the DOM using findDOMNode @@ -374,6 +381,7 @@ src/isomorphic/modern/class/__tests__/ReactES6Class-test.js * renders a simple stateless component with prop * renders based on state using initial values in this.props * renders based on state using props in the constructor +* renders based on context in the constructor * renders only once when setting state in componentWillMount * should throw with non-object in the initial state property * should render with null in the initial state property @@ -385,6 +393,7 @@ src/isomorphic/modern/class/__tests__/ReactES6Class-test.js * should warn when misspelling shouldComponentUpdate * should warn when misspelling componentWillReceiveProps * should throw AND warn when trying to access classic APIs +* supports this.context passed via getChildContext * supports classic refs * supports drilling through to the DOM using findDOMNode @@ -399,6 +408,7 @@ src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts * renders a simple stateless component with prop * renders based on state using initial values in this.props * renders based on state using props in the constructor +* renders based on context in the constructor * renders only once when setting state in componentWillMount * should throw with non-object in the initial state property * should render with null in the initial state property @@ -410,6 +420,7 @@ src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts * should warn when misspelling shouldComponentUpdate * should warn when misspelling componentWillReceiveProps * should throw AND warn when trying to access classic APIs +* supports this.context passed via getChildContext * supports classic refs * supports drilling through to the DOM using findDOMNode @@ -446,6 +457,7 @@ src/isomorphic/modern/element/__tests__/ReactJSXElementValidator-test.js * should not check the default for explicit null * should check declared prop types * should warn on invalid prop types +* should warn on invalid context types * should warn if getDefaultProps is specificed on the class src/renderers/dom/__tests__/ReactDOMProduction-test.js @@ -806,6 +818,7 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js * performs batched updates at the end of the batch * can nest batchedUpdates * can handle if setState callback throws +* merges and masks context src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js * catches render error in a boundary during mounting @@ -1018,7 +1031,10 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js * should call componentWillUnmount before unmounting * should warn when shouldComponentUpdate() returns undefined * should warn when componentDidUnmount method is defined +* should pass context to children when not owner * should skip update when rerendering element in container +* should pass context transitively +* should pass context when re-rendered * only renders once if updated in componentWillReceiveProps * should allow access to findDOMNode in componentWillUnmount * context should be passed down from the parent @@ -1147,9 +1163,11 @@ src/renderers/shared/stack/reconciler/__tests__/ReactStatelessComponent-test.js * should render stateless component * should update stateless component * should unmount stateless component +* should pass context thru stateless component * should provide a null ref * should use correct name in key warning * should support default props and prop types +* should receive context * should work with arrow functions * should allow simple functions to return null * should allow simple functions to return false From e6a0de54635ee003a29efa55e51dd6e97587d40e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 14 Nov 2016 00:30:22 +0000 Subject: [PATCH 04/14] Fix bad ops.length assignment in tests --- .../shared/fiber/__tests__/ReactIncremental-test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 48623edd5f1..763ddd61af5 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -1596,7 +1596,7 @@ describe('ReactIncremental', () => { } } - ops.length = []; + ops.length = 0; ReactNoop.render( @@ -1612,7 +1612,7 @@ describe('ReactIncremental', () => { 'ShowBoth {"locale":"fr"}', ]); - ops.length = []; + ops.length = 0; ReactNoop.render( @@ -1628,7 +1628,7 @@ describe('ReactIncremental', () => { 'ShowBoth {"locale":"de"}', ]); - ops.length = []; + ops.length = 0; ReactNoop.render( @@ -1642,7 +1642,7 @@ describe('ReactIncremental', () => { 'Intl null', ]); - ops.length = []; + ops.length = 0; ReactNoop.render( From 4ba0eb91586c1f9800ced9c73b94568ff906ddb6 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 14 Nov 2016 00:31:08 +0000 Subject: [PATCH 05/14] Add a test for recursive context --- .../fiber/__tests__/ReactIncremental-test.js | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 763ddd61af5..aaa0fdb302c 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -1667,4 +1667,35 @@ describe('ReactIncremental', () => { 'ShowBoth {"locale":"en"}', ]); }); + + it('does not leak own context into context provider', () => { + var ops = []; + class Recurse extends React.Component { + static contextTypes = { + n: React.PropTypes.number, + }; + static childContextTypes = { + n: React.PropTypes.number, + }; + getChildContext() { + return {n: (this.context.n || 3) - 1}; + } + render() { + ops.push('Recurse ' + JSON.stringify(this.context)); + if (this.context.n === 0) { + return null; + } + return ; + } + } + + ReactNoop.render(); + ReactNoop.flush(); + expect(ops).toEqual([ + 'Recurse {}', + 'Recurse {"n":2}', + 'Recurse {"n":1}', + 'Recurse {"n":0}', + ]); + }); }); From c22b7a001cc7e1a0f75e4f4826782bce1949ffee Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 14 Nov 2016 00:32:02 +0000 Subject: [PATCH 06/14] Add a failing test for context when reusing work We mistakingly forget to push the context if the parent is being reused but the child is not. --- .../fiber/__tests__/ReactIncremental-test.js | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index aaa0fdb302c..96c1619d005 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -1698,4 +1698,62 @@ describe('ReactIncremental', () => { 'Recurse {"n":0}', ]); }); + + it('provides context when reusing work', () => { + var ops = []; + + class Intl extends React.Component { + static childContextTypes = { + locale: React.PropTypes.string, + }; + getChildContext() { + return { + locale: this.props.locale, + }; + } + render() { + ops.push('Intl ' + JSON.stringify(this.context)); + return this.props.children; + } + } + + class ShowLocale extends React.Component { + static contextTypes = { + locale: React.PropTypes.string, + }; + + render() { + ops.push('ShowLocale ' + JSON.stringify(this.context)); + return this.context.locale; + } + } + + ops.length = 0; + ReactNoop.render( + + + + + + ); + ReactNoop.flushDeferredPri(40); + expect(ops).toEqual([ + 'Intl null', + 'ShowLocale {"locale":"fr"}', + 'ShowLocale {"locale":"fr"}', + ]); + + ops.length = 0; + ReactNoop.flush(); + expect(ops).toEqual([ + 'ShowLocale {"locale":"fr"}', + 'Intl null', + 'ShowLocale {"locale":"ru"}', + ]); + }); }); From f9e1bc9c97251e5593d102f0e553b8af4ad0865a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 14 Nov 2016 00:34:12 +0000 Subject: [PATCH 07/14] Move context management into scheduler It is error-prone to push and pop context in begin or complete phases because of all the bailouts. Scheduler has enough knowledge to see if pushing is necessary because it knows when we go inside or outside the tree. --- scripts/fiber/tests-passing.txt | 2 + .../shared/fiber/ReactFiberBeginWork.js | 10 +---- .../shared/fiber/ReactFiberCompleteWork.js | 38 ++++++------------- .../shared/fiber/ReactFiberContext.js | 16 ++++++-- .../shared/fiber/ReactFiberScheduler.js | 24 +++++++++++- 5 files changed, 50 insertions(+), 40 deletions(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 4069558127a..bae90866525 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -819,6 +819,8 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js * can nest batchedUpdates * can handle if setState callback throws * merges and masks context +* does not leak own context into context provider +* provides context when reusing work src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js * catches render error in a boundary during mounting diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 55f7128f524..1c4bd5a8e52 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -27,8 +27,6 @@ var { var ReactTypeOfWork = require('ReactTypeOfWork'); var { getMaskedContext, - pushContextProvider, - resetContext, } = require('ReactFiberContext'); var { IndeterminateComponent, @@ -189,14 +187,11 @@ module.exports = function( } else { shouldUpdate = updateClassInstance(current, workInProgress); } - const instance = workInProgress.stateNode; - if (typeof instance.getChildContext === 'function') { - pushContextProvider(workInProgress); - } if (!shouldUpdate) { return bailoutOnAlreadyFinishedWork(current, workInProgress); } // Rerender + const instance = workInProgress.stateNode; ReactCurrentOwner.current = workInProgress; const nextChildren = instance.render(); reconcileChildren(current, workInProgress, nextChildren); @@ -367,9 +362,6 @@ module.exports = function( } function beginWork(current : ?Fiber, workInProgress : Fiber, priorityLevel : PriorityLevel) : ?Fiber { - if (!workInProgress.return) { - resetContext(); - } if (workInProgress.pendingWorkPriority === NoWork || workInProgress.pendingWorkPriority > priorityLevel) { return bailoutOnLowPriority(current, workInProgress); diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 1444581805e..b0eea7c87e2 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -18,7 +18,6 @@ import type { HostConfig } from 'ReactFiberReconciler'; import type { ReifiedYield } from 'ReactReifiedYield'; var { reconcileChildFibers } = require('ReactChildFiber'); -var { popContextProvider } = require('ReactFiberContext'); var ReactTypeOfWork = require('ReactTypeOfWork'); var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect'); var { @@ -121,11 +120,10 @@ module.exports = function(config : HostConfig) { function completeWork(current : ?Fiber, workInProgress : Fiber) : ?Fiber { switch (workInProgress.tag) { - case FunctionalComponent: { + case FunctionalComponent: transferOutput(workInProgress.child, workInProgress); return null; - } - case ClassComponent: { + case ClassComponent: transferOutput(workInProgress.child, workInProgress); // Don't use the state queue to compute the memoized state. We already // merged it and assigned it to the instance. Transfer it from there. @@ -150,13 +148,8 @@ module.exports = function(config : HostConfig) { workInProgress.callbackList = updateQueue; markCallback(workInProgress); } - const instance = workInProgress.stateNode; - if (typeof instance.getChildContext === 'function') { - popContextProvider(); - } return null; - } - case HostContainer: { + case HostContainer: transferOutput(workInProgress.child, workInProgress); // We don't know if a container has updated any children so we always // need to update it right now. We schedule this side-effect before @@ -165,8 +158,7 @@ module.exports = function(config : HostConfig) { // are invoked. markUpdate(workInProgress); return null; - } - case HostComponent: { + case HostComponent: let newProps = workInProgress.pendingProps; if (current && workInProgress.stateNode != null) { // If we have an alternate, that means this is an update and we need to @@ -208,8 +200,7 @@ module.exports = function(config : HostConfig) { } workInProgress.memoizedProps = newProps; return null; - } - case HostText: { + case HostText: let newText = workInProgress.pendingProps; if (current && workInProgress.stateNode != null) { // If we have an alternate, that means this is an update and we need to @@ -231,32 +222,25 @@ module.exports = function(config : HostConfig) { } workInProgress.memoizedProps = newText; return null; - } - case CoroutineComponent: { + case CoroutineComponent: return moveCoroutineToHandlerPhase(current, workInProgress); - } - case CoroutineHandlerPhase: { + case CoroutineHandlerPhase: transferOutput(workInProgress.stateNode, workInProgress); // Reset the tag to now be a first phase coroutine. workInProgress.tag = CoroutineComponent; return null; - } - case YieldComponent: { + case YieldComponent: // Does nothing. return null; - } - case Fragment: { + case Fragment: transferOutput(workInProgress.child, workInProgress); return null; - } // Error cases - case IndeterminateComponent: { + case IndeterminateComponent: throw new Error('An indeterminate component should have become determinate before completing.'); - } - default: { + default: throw new Error('Unknown unit of work tag'); - } } } diff --git a/src/renderers/shared/fiber/ReactFiberContext.js b/src/renderers/shared/fiber/ReactFiberContext.js index 4aa9310e2b7..eeb78e4f300 100644 --- a/src/renderers/shared/fiber/ReactFiberContext.js +++ b/src/renderers/shared/fiber/ReactFiberContext.js @@ -19,6 +19,9 @@ var invariant = require('invariant'); var { getComponentName, } = require('ReactFiberTreeReflection'); +var { + ClassComponent, +} = require('ReactTypeOfWork'); if (__DEV__) { var checkReactTypeSpec = require('checkReactTypeSpec'); @@ -56,12 +59,19 @@ exports.getMaskedContext = function(fiber : Fiber) { return context; }; -exports.popContextProvider = function() { +exports.isContextProvider = function(fiber : Fiber) : boolean { + return ( + fiber.tag === ClassComponent && + typeof fiber.stateNode.getChildContext === 'function' + ); +}; + +exports.popContextProvider = function() : void { stack[index] = emptyObject; index--; }; -exports.pushContextProvider = function(fiber : Fiber) { +exports.pushContextProvider = function(fiber : Fiber) : void { const instance = fiber.stateNode; const childContextTypes = fiber.type.childContextTypes; const childContext = instance.getChildContext(); @@ -85,7 +95,7 @@ exports.pushContextProvider = function(fiber : Fiber) { stack[index] = mergedContext; }; -exports.resetContext = function() { +exports.resetContext = function() : void { index = -1; }; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 71aff51a26b..3a4e05081d0 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -24,6 +24,13 @@ var ReactCurrentOwner = require('ReactCurrentOwner'); var { cloneFiber } = require('ReactFiber'); +var { + isContextProvider, + pushContextProvider, + popContextProvider, + resetContext, +} = require('ReactFiberContext'); + var { NoWork, SynchronousPriority, @@ -267,6 +274,11 @@ module.exports = function(config : HostConfig) { const current = workInProgress.alternate; const next = completeWork(current, workInProgress); + // We are leaving this subtree, so pop context if any. + if (isContextProvider(workInProgress)) { + popContextProvider(); + } + resetWorkPriority(workInProgress); // The work is now done. We don't need this anymore. This flags @@ -345,6 +357,11 @@ module.exports = function(config : HostConfig) { } function performUnitOfWork(workInProgress : Fiber) : ?Fiber { + if (!workInProgress.return) { + // Don't start new work with context on the stack. + resetContext(); + } + // The current, flushed, state of this fiber is the alternate. // Ideally nothing should rely on this, but relying on it here // means that we don't need an additional field on the work in @@ -360,7 +377,12 @@ module.exports = function(config : HostConfig) { ReactFiberInstrumentation.debugTool.onDidBeginWork(workInProgress); } - if (!next) { + if (next) { + // There is work deeper in the tree, so push the context if it exists. + if (isContextProvider(workInProgress)) { + pushContextProvider(workInProgress); + } + } else { if (__DEV__ && ReactFiberInstrumentation.debugTool) { ReactFiberInstrumentation.debugTool.onWillCompleteWork(workInProgress); } From 56e1c126c5c29339f8bad4164f8ee510b79a6ea4 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 14 Nov 2016 16:19:03 +0000 Subject: [PATCH 08/14] Move context handling back into Begin and Complete phases As per discussion, this is better because then this code only runs for the class type of work. We will still need to fix bailouts for deep setState calls. --- .../shared/fiber/ReactFiberBeginWork.js | 16 +++++++++++++ .../shared/fiber/ReactFiberCompleteWork.js | 8 +++++++ .../shared/fiber/ReactFiberScheduler.js | 24 +------------------ 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 1c4bd5a8e52..155f951b61a 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -27,6 +27,9 @@ var { var ReactTypeOfWork = require('ReactTypeOfWork'); var { getMaskedContext, + isContextProvider, + pushContextProvider, + resetContext, } = require('ReactFiberContext'); var { IndeterminateComponent, @@ -195,6 +198,10 @@ module.exports = function( ReactCurrentOwner.current = workInProgress; const nextChildren = instance.render(); reconcileChildren(current, workInProgress, nextChildren); + // Put context on the stack because we will work on children + if (isContextProvider(workInProgress)) { + pushContextProvider(workInProgress); + } return workInProgress.child; } @@ -352,6 +359,10 @@ module.exports = function( cloneChildFibers(current, workInProgress); markChildAsProgressed(current, workInProgress, priorityLevel); + // Put context on the stack because we will work on children + if (isContextProvider(workInProgress)) { + pushContextProvider(workInProgress); + } return workInProgress.child; } @@ -362,6 +373,11 @@ module.exports = function( } function beginWork(current : ?Fiber, workInProgress : Fiber, priorityLevel : PriorityLevel) : ?Fiber { + if (!workInProgress.return) { + // Don't start new work with context on the stack. + resetContext(); + } + if (workInProgress.pendingWorkPriority === NoWork || workInProgress.pendingWorkPriority > priorityLevel) { return bailoutOnLowPriority(current, workInProgress); diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index b0eea7c87e2..82a9bdf211a 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -18,6 +18,10 @@ import type { HostConfig } from 'ReactFiberReconciler'; import type { ReifiedYield } from 'ReactReifiedYield'; var { reconcileChildFibers } = require('ReactChildFiber'); +var { + isContextProvider, + popContextProvider, +} = require('ReactFiberContext'); var ReactTypeOfWork = require('ReactTypeOfWork'); var ReactTypeOfSideEffect = require('ReactTypeOfSideEffect'); var { @@ -125,6 +129,10 @@ module.exports = function(config : HostConfig) { return null; case ClassComponent: transferOutput(workInProgress.child, workInProgress); + // We are leaving this subtree, so pop context if any. + if (isContextProvider(workInProgress)) { + popContextProvider(); + } // Don't use the state queue to compute the memoized state. We already // merged it and assigned it to the instance. Transfer it from there. // Also need to transfer the props, because pendingProps will be null diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 3a4e05081d0..71aff51a26b 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -24,13 +24,6 @@ var ReactCurrentOwner = require('ReactCurrentOwner'); var { cloneFiber } = require('ReactFiber'); -var { - isContextProvider, - pushContextProvider, - popContextProvider, - resetContext, -} = require('ReactFiberContext'); - var { NoWork, SynchronousPriority, @@ -274,11 +267,6 @@ module.exports = function(config : HostConfig) { const current = workInProgress.alternate; const next = completeWork(current, workInProgress); - // We are leaving this subtree, so pop context if any. - if (isContextProvider(workInProgress)) { - popContextProvider(); - } - resetWorkPriority(workInProgress); // The work is now done. We don't need this anymore. This flags @@ -357,11 +345,6 @@ module.exports = function(config : HostConfig) { } function performUnitOfWork(workInProgress : Fiber) : ?Fiber { - if (!workInProgress.return) { - // Don't start new work with context on the stack. - resetContext(); - } - // The current, flushed, state of this fiber is the alternate. // Ideally nothing should rely on this, but relying on it here // means that we don't need an additional field on the work in @@ -377,12 +360,7 @@ module.exports = function(config : HostConfig) { ReactFiberInstrumentation.debugTool.onDidBeginWork(workInProgress); } - if (next) { - // There is work deeper in the tree, so push the context if it exists. - if (isContextProvider(workInProgress)) { - pushContextProvider(workInProgress); - } - } else { + if (!next) { if (__DEV__ && ReactFiberInstrumentation.debugTool) { ReactFiberInstrumentation.debugTool.onWillCompleteWork(workInProgress); } From eedca6f641d2ea52fb1de5f12f61bc21b7c5f0d6 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Mon, 14 Nov 2016 18:24:30 +0000 Subject: [PATCH 09/14] Disable memoized props bailout when context might have changed --- scripts/fiber/tests-failing.txt | 2 -- scripts/fiber/tests-passing.txt | 2 ++ .../shared/fiber/ReactFiberBeginWork.js | 8 +++++--- src/renderers/shared/fiber/ReactFiberContext.js | 17 ++++++++++++----- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 3367df108b4..af870d39a1c 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -425,8 +425,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js * should warn about `setState` on unmounted components * should warn about `setState` in render * should warn about `setState` in getChildContext -* should pass context when re-rendered for static child -* should pass context when re-rendered for static child within a composite component * unmasked context propagates through updates * should trigger componentWillReceiveProps for context changes * should update refs if shouldComponentUpdate gives false diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index bae90866525..c763f784ea4 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1035,6 +1035,8 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js * should warn when componentDidUnmount method is defined * should pass context to children when not owner * should skip update when rerendering element in container +* should pass context when re-rendered for static child +* should pass context when re-rendered for static child within a composite component * should pass context transitively * should pass context when re-rendered * only renders once if updated in componentWillReceiveProps diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 155f951b61a..02fd265aa93 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -28,6 +28,7 @@ var ReactTypeOfWork = require('ReactTypeOfWork'); var { getMaskedContext, isContextProvider, + hasContextChanged, pushContextProvider, resetContext, } = require('ReactFiberContext'); @@ -200,7 +201,7 @@ module.exports = function( reconcileChildren(current, workInProgress, nextChildren); // Put context on the stack because we will work on children if (isContextProvider(workInProgress)) { - pushContextProvider(workInProgress); + pushContextProvider(workInProgress, true); } return workInProgress.child; } @@ -361,7 +362,7 @@ module.exports = function( markChildAsProgressed(current, workInProgress, priorityLevel); // Put context on the stack because we will work on children if (isContextProvider(workInProgress)) { - pushContextProvider(workInProgress); + pushContextProvider(workInProgress, false); } return workInProgress.child; } @@ -398,7 +399,8 @@ module.exports = function( workInProgress.memoizedProps !== null && workInProgress.pendingProps === workInProgress.memoizedProps )) && - workInProgress.updateQueue === null) { + workInProgress.updateQueue === null && + !hasContextChanged()) { return bailoutOnAlreadyFinishedWork(current, workInProgress); } diff --git a/src/renderers/shared/fiber/ReactFiberContext.js b/src/renderers/shared/fiber/ReactFiberContext.js index eeb78e4f300..fbc478020fe 100644 --- a/src/renderers/shared/fiber/ReactFiberContext.js +++ b/src/renderers/shared/fiber/ReactFiberContext.js @@ -28,13 +28,14 @@ if (__DEV__) { } let index = -1; -const stack = []; +const contextStack : Array = []; +const didPerformWorkStack : Array = []; function getUnmaskedContext() { if (index === -1) { return emptyObject; } - return stack[index]; + return contextStack[index]; } exports.getMaskedContext = function(fiber : Fiber) { @@ -59,6 +60,10 @@ exports.getMaskedContext = function(fiber : Fiber) { return context; }; +exports.hasContextChanged = function() : boolean { + return index > -1 && didPerformWorkStack[index]; +}; + exports.isContextProvider = function(fiber : Fiber) : boolean { return ( fiber.tag === ClassComponent && @@ -67,11 +72,12 @@ exports.isContextProvider = function(fiber : Fiber) : boolean { }; exports.popContextProvider = function() : void { - stack[index] = emptyObject; + contextStack[index] = emptyObject; + didPerformWorkStack[index] = false; index--; }; -exports.pushContextProvider = function(fiber : Fiber) : void { +exports.pushContextProvider = function(fiber : Fiber, didPerformWork : boolean) : void { const instance = fiber.stateNode; const childContextTypes = fiber.type.childContextTypes; const childContext = instance.getChildContext(); @@ -92,7 +98,8 @@ exports.pushContextProvider = function(fiber : Fiber) : void { const mergedContext = Object.assign({}, getUnmaskedContext(), childContext); index++; - stack[index] = mergedContext; + contextStack[index] = mergedContext; + didPerformWorkStack[index] = didPerformWork; }; exports.resetContext = function() : void { From 299009c41fb6a2dd92247f806bef32859e2b9336 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 15 Nov 2016 13:22:41 +0000 Subject: [PATCH 10/14] Add test cases for context below and above setState --- .../fiber/__tests__/ReactIncremental-test.js | 151 +++++++++++++++++- 1 file changed, 148 insertions(+), 3 deletions(-) diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 96c1619d005..14cc1452548 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -1547,7 +1547,6 @@ describe('ReactIncremental', () => { static contextTypes = { locale: React.PropTypes.string, }; - render() { ops.push('ShowLocale ' + JSON.stringify(this.context)); return this.context.locale; @@ -1558,7 +1557,6 @@ describe('ReactIncremental', () => { static contextTypes = { route: React.PropTypes.string, }; - render() { ops.push('ShowRoute ' + JSON.stringify(this.context)); return this.context.route; @@ -1721,7 +1719,6 @@ describe('ReactIncremental', () => { static contextTypes = { locale: React.PropTypes.string, }; - render() { ops.push('ShowLocale ' + JSON.stringify(this.context)); return this.context.locale; @@ -1756,4 +1753,152 @@ describe('ReactIncremental', () => { 'ShowLocale {"locale":"ru"}', ]); }); + + it('reads context when setState is below the provider', () => { + var ops = []; + var statefulInst; + + class Intl extends React.Component { + static childContextTypes = { + locale: React.PropTypes.string, + }; + getChildContext() { + const childContext = { + locale: this.props.locale, + }; + ops.push('Intl:provide ' + JSON.stringify(childContext)); + return childContext; + } + render() { + ops.push('Intl:read ' + JSON.stringify(this.context)); + return this.props.children; + } + } + + class ShowLocaleClass extends React.Component { + static contextTypes = { + locale: React.PropTypes.string, + }; + render() { + ops.push('ShowLocaleClass:read ' + JSON.stringify(this.context)); + return this.context.locale; + } + } + + function ShowLocaleFn(props, context) { + ops.push('ShowLocaleFn:read ' + JSON.stringify(context)); + return context.locale; + } + ShowLocaleFn.contextTypes = { + locale: React.PropTypes.string, + }; + + class Stateful extends React.Component { + state = {x: 0}; + render() { + statefulInst = this; + return [ + , + , + ]; + } + } + + ops.length = 0; + ReactNoop.render( + + + + ); + ReactNoop.flush(); + expect(ops).toEqual([ + 'Intl:read null', + 'Intl:provide {"locale":"fr"}', + 'ShowLocaleClass:read {"locale":"fr"}', + 'ShowLocaleFn:read {"locale":"fr"}', + ]); + + ops.length = 0; + statefulInst.setState({x: 1}); + ReactNoop.flush(); + expect(ops).toEqual([ + // TODO: we should be able to reuse the previous child context. + 'Intl:provide {"locale":"fr"}', + 'ShowLocaleClass:read {"locale":"fr"}', + 'ShowLocaleFn:read {"locale":"fr"}', + ]); + }); + + it('reads context when setState is above the provider', () => { + var ops = []; + var statefulInst; + + class Intl extends React.Component { + static childContextTypes = { + locale: React.PropTypes.string, + }; + getChildContext() { + const childContext = { + locale: this.props.locale, + }; + ops.push('Intl:provide ' + JSON.stringify(childContext)); + return childContext; + } + render() { + ops.push('Intl:read ' + JSON.stringify(this.context)); + return this.props.children; + } + } + + class ShowLocaleClass extends React.Component { + static contextTypes = { + locale: React.PropTypes.string, + }; + render() { + ops.push('ShowLocaleClass:read ' + JSON.stringify(this.context)); + return this.context.locale; + } + } + + function ShowLocaleFn(props, context) { + ops.push('ShowLocaleFn:read ' + JSON.stringify(context)); + return context.locale; + } + ShowLocaleFn.contextTypes = { + locale: React.PropTypes.string, + }; + + class Stateful extends React.Component { + state = {locale: 'fr'}; + render() { + statefulInst = this; + return ( + + + + + ); + } + } + + ops.length = 0; + ReactNoop.render(); + ReactNoop.flush(); + expect(ops).toEqual([ + 'Intl:read null', + 'Intl:provide {"locale":"fr"}', + 'ShowLocaleClass:read {"locale":"fr"}', + 'ShowLocaleFn:read {"locale":"fr"}', + ]); + + ops.length = 0; + statefulInst.setState({locale: 'gr'}); + ReactNoop.flush(); + expect(ops).toEqual([ + 'Intl:read null', + 'Intl:provide {"locale":"gr"}', + 'ShowLocaleClass:read {"locale":"gr"}', + 'ShowLocaleFn:read {"locale":"gr"}', + ]); + }); }); From a6ee5b876a90e87efe9fe2f3605bef9347fa4327 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 15 Nov 2016 14:41:28 +0000 Subject: [PATCH 11/14] Memoize merged child context when possible We can avoid recreating the merged context object if the context provider work was reused. This is essential to avoid allocations for deep setState() calls. --- .../shared/fiber/ReactFiberContext.js | 39 ++++++++++++------- .../fiber/__tests__/ReactIncremental-test.js | 7 +++- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/renderers/shared/fiber/ReactFiberContext.js b/src/renderers/shared/fiber/ReactFiberContext.js index fbc478020fe..5fd47f5a18c 100644 --- a/src/renderers/shared/fiber/ReactFiberContext.js +++ b/src/renderers/shared/fiber/ReactFiberContext.js @@ -80,23 +80,32 @@ exports.popContextProvider = function() : void { exports.pushContextProvider = function(fiber : Fiber, didPerformWork : boolean) : void { const instance = fiber.stateNode; const childContextTypes = fiber.type.childContextTypes; - const childContext = instance.getChildContext(); - - for (let contextKey in childContext) { - invariant( - contextKey in childContextTypes, - '%s.getChildContext(): key "%s" is not defined in childContextTypes.', - getComponentName(fiber), - contextKey - ); - } - if (__DEV__) { - const name = getComponentName(fiber); - const debugID = 0; // TODO: pass a real ID - checkReactTypeSpec(childContextTypes, childContext, 'childContext', name, null, debugID); + + const memoizedMergedChildContext = instance.__reactInternalMemoizedMergedChildContext; + const canReuseMergedChildContext = !didPerformWork && memoizedMergedChildContext != null; + + let mergedContext = null; + if (canReuseMergedChildContext) { + mergedContext = memoizedMergedChildContext; + } else { + const childContext = instance.getChildContext(); + for (let contextKey in childContext) { + invariant( + contextKey in childContextTypes, + '%s.getChildContext(): key "%s" is not defined in childContextTypes.', + getComponentName(fiber), + contextKey + ); + } + if (__DEV__) { + const name = getComponentName(fiber); + const debugID = 0; // TODO: pass a real ID + checkReactTypeSpec(childContextTypes, childContext, 'childContext', name, null, debugID); + } + mergedContext = {...getUnmaskedContext(), ...childContext}; + instance.__reactInternalMemoizedMergedChildContext = mergedContext; } - const mergedContext = Object.assign({}, getUnmaskedContext(), childContext); index++; contextStack[index] = mergedContext; didPerformWorkStack[index] = didPerformWork; diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 14cc1452548..6c5e04d0104 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -1822,8 +1822,8 @@ describe('ReactIncremental', () => { statefulInst.setState({x: 1}); ReactNoop.flush(); expect(ops).toEqual([ - // TODO: we should be able to reuse the previous child context. - 'Intl:provide {"locale":"fr"}', + // Intl was memoized so we did not need to + // either render it or recompute its context. 'ShowLocaleClass:read {"locale":"fr"}', 'ShowLocaleFn:read {"locale":"fr"}', ]); @@ -1895,6 +1895,9 @@ describe('ReactIncremental', () => { statefulInst.setState({locale: 'gr'}); ReactNoop.flush(); expect(ops).toEqual([ + // Intl is below setState() so it might have been + // affected by it. Therefore we re-render and recompute + // its child context. 'Intl:read null', 'Intl:provide {"locale":"gr"}', 'ShowLocaleClass:read {"locale":"gr"}', From fd8c8038f01b992a81d86299b3e46bb5ab5f19a2 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 15 Nov 2016 14:49:54 +0000 Subject: [PATCH 12/14] Update passing tests --- scripts/fiber/tests-passing.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index c763f784ea4..c18c1d67f51 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -821,6 +821,8 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js * merges and masks context * does not leak own context into context provider * provides context when reusing work +* reads context when setState is below the provider +* reads context when setState is above the provider src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js * catches render error in a boundary during mounting From 1e42c1833cf50f2450be35758470f8b84d7a60a2 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 15 Nov 2016 16:08:15 +0000 Subject: [PATCH 13/14] Add explicit tests for intermediate components --- .../fiber/__tests__/ReactIncremental-test.js | 70 +++++++++++++++---- 1 file changed, 56 insertions(+), 14 deletions(-) diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 6c5e04d0104..8643829a71b 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -1797,23 +1797,41 @@ describe('ReactIncremental', () => { state = {x: 0}; render() { statefulInst = this; - return [ - , - , - ]; + return this.props.children; + } + } + + function IndirectionFn(props, context) { + ops.push('IndirectionFn ' + JSON.stringify(context)); + return props.children; + } + + class IndirectionClass extends React.Component { + render() { + ops.push('IndirectionClass ' + JSON.stringify(this.context)); + return this.props.children; } } ops.length = 0; ReactNoop.render( - + + + + + + + + ); ReactNoop.flush(); expect(ops).toEqual([ 'Intl:read null', 'Intl:provide {"locale":"fr"}', + 'IndirectionFn null', + 'IndirectionClass null', 'ShowLocaleClass:read {"locale":"fr"}', 'ShowLocaleFn:read {"locale":"fr"}', ]); @@ -1821,12 +1839,9 @@ describe('ReactIncremental', () => { ops.length = 0; statefulInst.setState({x: 1}); ReactNoop.flush(); - expect(ops).toEqual([ - // Intl was memoized so we did not need to - // either render it or recompute its context. - 'ShowLocaleClass:read {"locale":"fr"}', - 'ShowLocaleFn:read {"locale":"fr"}', - ]); + // All work has been memoized because setState() + // happened below the context and could not have affected it. + expect(ops).toEqual([]); }); it('reads context when setState is above the provider', () => { @@ -1868,25 +1883,47 @@ describe('ReactIncremental', () => { locale: React.PropTypes.string, }; + function IndirectionFn(props, context) { + ops.push('IndirectionFn ' + JSON.stringify(context)); + return props.children; + } + + class IndirectionClass extends React.Component { + render() { + ops.push('IndirectionClass ' + JSON.stringify(this.context)); + return this.props.children; + } + } + class Stateful extends React.Component { state = {locale: 'fr'}; render() { statefulInst = this; return ( - - + {this.props.children} ); } } ops.length = 0; - ReactNoop.render(); + ReactNoop.render( + + + + + + + + + ); ReactNoop.flush(); expect(ops).toEqual([ 'Intl:read null', 'Intl:provide {"locale":"fr"}', + 'IndirectionFn null', + 'IndirectionClass null', 'ShowLocaleClass:read {"locale":"fr"}', 'ShowLocaleFn:read {"locale":"fr"}', ]); @@ -1900,6 +1937,11 @@ describe('ReactIncremental', () => { // its child context. 'Intl:read null', 'Intl:provide {"locale":"gr"}', + // TODO: it's unfortunate that we can't reuse work on + // these components even though they don't depend on context. + 'IndirectionFn null', + 'IndirectionClass null', + // These components depend on context: 'ShowLocaleClass:read {"locale":"gr"}', 'ShowLocaleFn:read {"locale":"gr"}', ]); From 2397f1fce6082591be7c319c709906e0fa4426ae Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 15 Nov 2016 16:34:58 +0000 Subject: [PATCH 14/14] Use empty object if context does not exist This matches Stack behavior. Had to rewrite a test that used Simulate because Fiber doesn't support events yet. Also changed tests for componentDidUpdate() since Fiber intentionally doesn't pass prevContext argument. --- scripts/fiber/tests-failing.txt | 2 - scripts/fiber/tests-passing.txt | 2 + .../shared/fiber/ReactFiberContext.js | 2 +- .../fiber/__tests__/ReactIncremental-test.js | 38 +++++++++---------- .../__tests__/ReactCompositeComponent-test.js | 27 +++++++------ 5 files changed, 38 insertions(+), 33 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index af870d39a1c..bcd6e684cf6 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -425,8 +425,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js * should warn about `setState` on unmounted components * should warn about `setState` in render * should warn about `setState` in getChildContext -* unmasked context propagates through updates -* should trigger componentWillReceiveProps for context changes * should update refs if shouldComponentUpdate gives false * should support objects with prototypes as state diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index c18c1d67f51..9c2105a8ef9 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1041,6 +1041,8 @@ src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js * should pass context when re-rendered for static child within a composite component * should pass context transitively * should pass context when re-rendered +* unmasked context propagates through updates +* should trigger componentWillReceiveProps for context changes * only renders once if updated in componentWillReceiveProps * should allow access to findDOMNode in componentWillUnmount * context should be passed down from the parent diff --git a/src/renderers/shared/fiber/ReactFiberContext.js b/src/renderers/shared/fiber/ReactFiberContext.js index 5fd47f5a18c..d2e09311abc 100644 --- a/src/renderers/shared/fiber/ReactFiberContext.js +++ b/src/renderers/shared/fiber/ReactFiberContext.js @@ -42,7 +42,7 @@ exports.getMaskedContext = function(fiber : Fiber) { const type = fiber.type; const contextTypes = type.contextTypes; if (!contextTypes) { - return null; + return emptyObject; } const unmaskedContext = getUnmaskedContext(); diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js index 8643829a71b..65ff444e95a 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js @@ -1605,7 +1605,7 @@ describe('ReactIncremental', () => { ); ReactNoop.flush(); expect(ops).toEqual([ - 'Intl null', + 'Intl {}', 'ShowLocale {"locale":"fr"}', 'ShowBoth {"locale":"fr"}', ]); @@ -1621,7 +1621,7 @@ describe('ReactIncremental', () => { ); ReactNoop.flush(); expect(ops).toEqual([ - 'Intl null', + 'Intl {}', 'ShowLocale {"locale":"de"}', 'ShowBoth {"locale":"de"}', ]); @@ -1637,7 +1637,7 @@ describe('ReactIncremental', () => { ); ReactNoop.flushDeferredPri(15); expect(ops).toEqual([ - 'Intl null', + 'Intl {}', ]); ops.length = 0; @@ -1652,14 +1652,14 @@ describe('ReactIncremental', () => { ); ReactNoop.flush(); expect(ops).toEqual([ - 'Intl null', + 'Intl {}', 'ShowLocale {"locale":"en"}', - 'Router null', - 'Indirection null', + 'Router {}', + 'Indirection {}', 'ShowLocale {"locale":"en"}', 'ShowRoute {"route":"/about"}', - 'ShowNeither null', - 'Intl null', + 'ShowNeither {}', + 'Intl {}', 'ShowBoth {"locale":"ru","route":"/about"}', 'ShowBoth {"locale":"en","route":"/about"}', 'ShowBoth {"locale":"en"}', @@ -1740,7 +1740,7 @@ describe('ReactIncremental', () => { ); ReactNoop.flushDeferredPri(40); expect(ops).toEqual([ - 'Intl null', + 'Intl {}', 'ShowLocale {"locale":"fr"}', 'ShowLocale {"locale":"fr"}', ]); @@ -1749,7 +1749,7 @@ describe('ReactIncremental', () => { ReactNoop.flush(); expect(ops).toEqual([ 'ShowLocale {"locale":"fr"}', - 'Intl null', + 'Intl {}', 'ShowLocale {"locale":"ru"}', ]); }); @@ -1828,10 +1828,10 @@ describe('ReactIncremental', () => { ); ReactNoop.flush(); expect(ops).toEqual([ - 'Intl:read null', + 'Intl:read {}', 'Intl:provide {"locale":"fr"}', - 'IndirectionFn null', - 'IndirectionClass null', + 'IndirectionFn {}', + 'IndirectionClass {}', 'ShowLocaleClass:read {"locale":"fr"}', 'ShowLocaleFn:read {"locale":"fr"}', ]); @@ -1920,10 +1920,10 @@ describe('ReactIncremental', () => { ); ReactNoop.flush(); expect(ops).toEqual([ - 'Intl:read null', + 'Intl:read {}', 'Intl:provide {"locale":"fr"}', - 'IndirectionFn null', - 'IndirectionClass null', + 'IndirectionFn {}', + 'IndirectionClass {}', 'ShowLocaleClass:read {"locale":"fr"}', 'ShowLocaleFn:read {"locale":"fr"}', ]); @@ -1935,12 +1935,12 @@ describe('ReactIncremental', () => { // Intl is below setState() so it might have been // affected by it. Therefore we re-render and recompute // its child context. - 'Intl:read null', + 'Intl:read {}', 'Intl:provide {"locale":"gr"}', // TODO: it's unfortunate that we can't reuse work on // these components even though they don't depend on context. - 'IndirectionFn null', - 'IndirectionClass null', + 'IndirectionFn {}', + 'IndirectionClass {}', // These components depend on context: 'ShowLocaleClass:read {"locale":"gr"}', 'ShowLocaleFn:read {"locale":"gr"}', diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js index c8055754abc..b5f92fd0047 100644 --- a/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js +++ b/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js @@ -15,6 +15,7 @@ var ChildUpdates; var MorphingComponent; var React; var ReactDOM; +var ReactDOMFeatureFlags; var ReactCurrentOwner; var ReactPropTypes; var ReactServerRendering; @@ -26,6 +27,7 @@ describe('ReactCompositeComponent', () => { jest.resetModuleRegistry(); React = require('React'); ReactDOM = require('ReactDOM'); + ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); ReactCurrentOwner = require('ReactCurrentOwner'); ReactPropTypes = require('ReactPropTypes'); ReactTestUtils = require('ReactTestUtils'); @@ -830,7 +832,10 @@ describe('ReactCompositeComponent', () => { } componentDidUpdate(prevProps, prevState, prevContext) { - expect('foo' in prevContext).toBe(true); + if (!ReactDOMFeatureFlags.useFiber) { + // Fiber does not pass the previous context. + expect('foo' in prevContext).toBe(true); + } } shouldComponentUpdate(nextProps, nextState, nextContext) { @@ -849,7 +854,10 @@ describe('ReactCompositeComponent', () => { } componentDidUpdate(prevProps, prevState, prevContext) { - expect('foo' in prevContext).toBe(false); + if (!ReactDOMFeatureFlags.useFiber) { + // Fiber does not pass the previous context. + expect('foo' in prevContext).toBe(false); + } } shouldComponentUpdate(nextProps, nextState, nextContext) { @@ -971,21 +979,16 @@ describe('ReactCompositeComponent', () => { }; } - onClick = () => { - this.setState({ - foo: 'def', - }); - }; - render() { - return
{this.props.children}
; + return
{this.props.children}
; } } var div = document.createElement('div'); + var parentInstance = null; ReactDOM.render( - + parentInstance = inst}> A1 A2 @@ -999,7 +1002,9 @@ describe('ReactCompositeComponent', () => { div ); - ReactTestUtils.Simulate.click(div.childNodes[0]); + parentInstance.setState({ + foo: 'def', + }); expect(propChanges).toBe(0); expect(contextChanges).toBe(3); // ChildWithContext, GrandChild x 2