From bebc568ceb6d93dd5ded624842f9b7e9d9a84e8e Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 27 Jan 2015 01:19:31 -0800 Subject: [PATCH] Use Singleton LifeCycle State All entry points are for reconciliation are within batching strategies. Since we don't have any batching strategies with synchronous updates, there can't be more than one life-cycle method on the stack at any given time. Therefore, it's safe to move the composite life cycle flag to a singleton. This saves us some memory management. I think that we can get rid of these life cycle states completely in the future. --- src/classic/class/ReactClass.js | 8 +- src/core/ReactCompositeComponent.js | 78 +++++++++++-------- src/core/ReactLifeCycle.js | 57 ++++---------- src/core/ReactUpdateQueue.js | 10 +-- .../__tests__/ReactComponentLifeCycle-test.js | 69 +++++++++------- 5 files changed, 115 insertions(+), 107 deletions(-) diff --git a/src/classic/class/ReactClass.js b/src/classic/class/ReactClass.js index 5b9e5647bf..d0490a2a82 100644 --- a/src/classic/class/ReactClass.js +++ b/src/classic/class/ReactClass.js @@ -15,6 +15,7 @@ var ReactComponentBase = require('ReactComponentBase'); var ReactElement = require('ReactElement'); var ReactErrorUtils = require('ReactErrorUtils'); var ReactInstanceMap = require('ReactInstanceMap'); +var ReactLifeCycle = require('ReactLifeCycle'); var ReactPropTypeLocations = require('ReactPropTypeLocations'); var ReactPropTypeLocationNames = require('ReactPropTypeLocationNames'); var ReactUpdateQueue = require('ReactUpdateQueue'); @@ -734,9 +735,10 @@ var ReactClassMixin = { */ isMounted: function() { var internalInstance = ReactInstanceMap.get(this); - // In theory, isMounted is always true if it exists in the map. - // TODO: Remove the internal isMounted method. - return internalInstance && internalInstance.isMounted(); + return ( + internalInstance && + internalInstance !== ReactLifeCycle.currentlyMountingInstance + ); }, /** diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 8d5481a7c5..2a336836be 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -42,6 +42,33 @@ function getDeclarationErrorAddendum(component) { return ''; } +/** + * ------------------ The Life-Cycle of a Composite Component ------------------ + * + * - constructor: Initialization of state. The instance is now retained. + * - componentWillMount + * - render + * - [children's constructors] + * - [children's componentWillMount and render] + * - [children's componentDidMount] + * - componentDidMount + * + * Update Phases: + * - componentWillReceiveProps (only called if parent updated) + * - shouldComponentUpdate + * - componentWillUpdate + * - render + * - [children's constructors or receive props phases] + * - componentDidUpdate + * + * - componentWillUnmount + * - [children's componentWillUnmount] + * - [children destroyed] + * - (destroyed): The instance is now blank, released by React and ready for GC. + * + * ----------------------------------------------------------------------------- + */ + /** * An incrementing ID assigned to each component when it is mounted. This is * used to enforce the order in which `ReactUpdates` updates dirty components. @@ -71,7 +98,6 @@ var ReactCompositeComponentMixin = { this._pendingElement = null; this._pendingState = null; this._pendingForceUpdate = false; - this._compositeLifeCycleState = null; this._renderedComponent = null; @@ -83,16 +109,6 @@ var ReactCompositeComponentMixin = { this._pendingCallbacks = null; }, - /** - * Checks whether or not this composite component is mounted. - * @return {boolean} True if mounted, false otherwise. - * @protected - * @final - */ - isMounted: function() { - return this._compositeLifeCycleState !== ReactLifeCycle.MOUNTING; - }, - /** * Initializes the component, renders markup, and registers event listeners. * @@ -127,8 +143,6 @@ var ReactCompositeComponentMixin = { // Store a reference from the instance back to the internal representation ReactInstanceMap.set(inst, this); - this._compositeLifeCycleState = ReactLifeCycle.MOUNTING; - if (__DEV__) { this._warnIfContextsDiffer(this._currentElement._context, context); } @@ -176,13 +190,18 @@ var ReactCompositeComponentMixin = { '%s.state: must be set to an object or null', this.getName() || 'ReactCompositeComponent' ); - inst.state = initialState; this._pendingState = null; this._pendingForceUpdate = false; if (inst.componentWillMount) { - inst.componentWillMount(); + var previouslyMounting = ReactLifeCycle.currentlyMountingInstance; + ReactLifeCycle.currentlyMountingInstance = this; + try { + inst.componentWillMount(); + } finally { + ReactLifeCycle.currentlyMountingInstance = previouslyMounting; + } // When mounting, calls to `setState` by `componentWillMount` will set // `this._pendingState` without triggering a re-render. if (this._pendingState) { @@ -197,8 +216,6 @@ var ReactCompositeComponentMixin = { this._currentElement.type // The wrapping type ); - // Done with mounting, `setState` will now trigger UI changes. - this._compositeLifeCycleState = null; var markup = ReactReconciler.mountComponent( this._renderedComponent, rootID, @@ -221,11 +238,15 @@ var ReactCompositeComponentMixin = { unmountComponent: function() { var inst = this._instance; - this._compositeLifeCycleState = ReactLifeCycle.UNMOUNTING; if (inst.componentWillUnmount) { - inst.componentWillUnmount(); + var previouslyUnmounting = ReactLifeCycle.currentlyUnmountingInstance; + ReactLifeCycle.currentlyUnmountingInstance = this; + try { + inst.componentWillUnmount(); + } finally { + ReactLifeCycle.currentlyUnmountingInstance = previouslyUnmounting; + } } - this._compositeLifeCycleState = null; ReactReconciler.unmountComponent(this._renderedComponent); this._renderedComponent = null; @@ -238,6 +259,8 @@ var ReactCompositeComponentMixin = { ReactComponentEnvironment.unmountIDFromEnvironment(this._rootNodeID); + // These fields do not really need to be reset since this object is no + // longer accessible. this._context = null; this._rootNodeID = null; @@ -432,14 +455,6 @@ var ReactCompositeComponentMixin = { }, receiveComponent: function(nextElement, transaction, nextContext) { - var compositeLifeCycleState = this._compositeLifeCycleState; - // Do not trigger a state transition if we are in the middle of mounting or - // receiving props because both of those will already be doing this. - if (compositeLifeCycleState === ReactLifeCycle.MOUNTING || - compositeLifeCycleState === ReactLifeCycle.RECEIVING_PROPS) { - return; - } - var prevElement = this._currentElement; var prevContext = this._context; @@ -552,14 +567,15 @@ var ReactCompositeComponentMixin = { } } - this._compositeLifeCycleState = ReactLifeCycle.RECEIVING_PROPS; + // An update here will schedule an update but immediately set + // _pendingState which will ensure that any state updates gets + // immediately reconciled instead of waiting for the next batch. + if (inst.componentWillReceiveProps) { inst.componentWillReceiveProps(nextProps, nextContext); } } - this._compositeLifeCycleState = null; - var nextState = this._pendingState || inst.state; this._pendingState = null; diff --git a/src/core/ReactLifeCycle.js b/src/core/ReactLifeCycle.js index 81ae5c9bbd..cd6a2365af 100644 --- a/src/core/ReactLifeCycle.js +++ b/src/core/ReactLifeCycle.js @@ -11,50 +11,25 @@ "use strict"; -var keyMirror = require('keyMirror'); - /** - * `ReactCompositeComponent` maintains an auxiliary life cycle state in - * `this._compositeLifeCycleState` (which can be null). + * This module manages the bookkeeping when a component is in the process + * of being mounted or being unmounted. This is used as a way to enforce + * invariants (or warnings) when it is not recommended to call + * setState/forceUpdate. * - * This is different from the life cycle state maintained by `ReactComponent`. - * The following diagram shows how the states overlap in - * time. There are times when the CompositeLifeCycle is null - at those times it - * is only meaningful to look at ComponentLifeCycle alone. + * currentlyMountingInstance: During the construction phase, it is not possible + * to trigger an update since the instance is not fully mounted yet. However, we + * currently allow this as a convenience for mutating the initial state. * - * Top Row: ReactComponent.ComponentLifeCycle - * Low Row: ReactComponent.CompositeLifeCycle - * - * +-------+---------------------------------+--------+ - * | UN | MOUNTED | UN | - * |MOUNTED| | MOUNTED| - * +-------+---------------------------------+--------+ - * | ^--------+ +-------+ +--------^ | - * | | | | | | | | - * | 0--|MOUNTING|-0-|RECEIVE|-0-| UN |--->0 | - * | | | |PROPS | |MOUNTING| | - * | | | | | | | | - * | | | | | | | | - * | +--------+ +-------+ +--------+ | - * | | | | - * +-------+---------------------------------+--------+ + * currentlyUnmountingInstance: During the unmounting phase, the instance is + * still mounted and can therefore schedule an update. However, this is not + * recommended and probably an error since it's about to be unmounted. + * Therefore we still want to trigger in an error for that case. */ -var ReactLifeCycle = keyMirror({ - /** - * Components in the process of being mounted respond to state changes - * differently. - */ - MOUNTING: null, - /** - * Components in the process of being unmounted are guarded against state - * changes. - */ - UNMOUNTING: null, - /** - * Components that are mounted and receiving new props respond to state - * changes differently. - */ - RECEIVING_PROPS: null -}); + +var ReactLifeCycle = { + currentlyMountingInstance: null, + currentlyUnmountingInstance: null +}; module.exports = ReactLifeCycle; diff --git a/src/core/ReactUpdateQueue.js b/src/core/ReactUpdateQueue.js index 88fe04b55d..7562fea8cf 100644 --- a/src/core/ReactUpdateQueue.js +++ b/src/core/ReactUpdateQueue.js @@ -21,7 +21,7 @@ var assign = require('Object.assign'); var invariant = require('invariant'); function enqueueUpdate(internalInstance) { - if (internalInstance._compositeLifeCycleState !== ReactLifeCycle.MOUNTING) { + if (internalInstance !== ReactLifeCycle.currentlyMountingInstance) { // If we're in a componentWillMount handler, don't enqueue a rerender // because ReactUpdates assumes we're in a browser context (which is // wrong for server rendering) and we're about to do a render anyway. @@ -49,8 +49,7 @@ function getInternalInstanceReadyForUpdate(publicInstance, callerName) { callerName ); invariant( - internalInstance._compositeLifeCycleState !== - ReactLifeCycle.UNMOUNTING, + internalInstance !== ReactLifeCycle.currentlyUnmountingInstance, '%s(...): Cannot call %s() on an unmounting component.', callerName, callerName @@ -84,7 +83,7 @@ var ReactUpdateQueue = { internalInstance, 'Cannot enqueue a callback on an instance that is unmounted.' ); - if (internalInstance._compositeLifeCycleState === ReactLifeCycle.MOUNTING) { + if (internalInstance === ReactLifeCycle.currentlyMountingInstance) { // Ignore callbacks in componentWillMount. See enqueueUpdate. return; } @@ -95,7 +94,8 @@ var ReactUpdateQueue = { } // TODO: The callback here is ignored when setState is called from // componentWillMount. Either fix it or disallow doing so completely in - // favor of getInitialState. + // favor of getInitialState. Alternatively, we can disallow + // componentWillMount during server-side rendering. enqueueUpdate(internalInstance); }, diff --git a/src/core/__tests__/ReactComponentLifeCycle-test.js b/src/core/__tests__/ReactComponentLifeCycle-test.js index ab3223ed1e..c97f3e09ee 100644 --- a/src/core/__tests__/ReactComponentLifeCycle-test.js +++ b/src/core/__tests__/ReactComponentLifeCycle-test.js @@ -14,13 +14,9 @@ var keyMirror = require('keyMirror'); var React; -var ReactTestUtils; -var ReactCompositeComponent; +var ReactLifeCycle; var ReactInstanceMap; -var CompositeComponentLifeCycle; - -var getCompositeLifeCycle; -var getLifeCycleState; +var ReactTestUtils; var clone = function(o) { return JSON.parse(JSON.stringify(o)); @@ -84,6 +80,44 @@ var ComponentLifeCycle = keyMirror({ UNMOUNTED: null }); +/** + * Composite components can also be in one of these life cycles. + */ +var CompositeComponentLifeCycle = keyMirror({ + /** + * Mounted components have a DOM node representation and are capable of + * receiving new props. + */ + MOUNTING: null, + /** + * Unmounted components are inactive and cannot receive new props. + */ + UNMOUNTING: null +}); + +function getCompositeLifeCycle(instance) { + var internalInstance = ReactInstanceMap.get(instance); + if (!internalInstance) { + return null; + } + if (ReactLifeCycle.currentlyMountingInstance === internalInstance) { + return CompositeComponentLifeCycle.MOUNTING; + } + if (ReactLifeCycle.currentlyUnmountingInstance === internalInstance) { + return CompositeComponentLifeCycle.UNMOUNTING; + } + return null; +} + +function getLifeCycleState(instance) { + var internalInstance = ReactInstanceMap.get(instance); + // Once a component gets mounted, it has an internal instance, once it + // gets unmounted, it loses that internal instance. + return internalInstance ? + ComponentLifeCycle.MOUNTED : + ComponentLifeCycle.UNMOUNTED; +} + /** * TODO: We should make any setState calls fail in * `getInitialState` and `componentWillMount`. They will usually fail @@ -96,27 +130,8 @@ describe('ReactComponentLifeCycle', function() { require('mock-modules').dumpCache(); React = require('React'); ReactTestUtils = require('ReactTestUtils'); - ReactCompositeComponent = require('ReactCompositeComponent'); - CompositeComponentLifeCycle = require('ReactLifeCycle'); - + ReactLifeCycle = require('ReactLifeCycle'); ReactInstanceMap = require('ReactInstanceMap'); - - getCompositeLifeCycle = function(instance) { - var internalInstance = ReactInstanceMap.get(instance); - if (!internalInstance) { - return null; - } - return internalInstance._compositeLifeCycleState; - }; - - getLifeCycleState = function(instance) { - var internalInstance = ReactInstanceMap.get(instance); - // Once a component gets mounted, it has an internal instance, once it - // gets unmounted, it loses that internal instance. - return internalInstance ? - ComponentLifeCycle.MOUNTED : - ComponentLifeCycle.UNMOUNTED; - }; }); it('should not reuse an instance when it has been unmounted', function() { @@ -345,7 +360,7 @@ describe('ReactComponentLifeCycle', function() { ComponentLifeCycle.MOUNTED ); expect(instance._testJournal.compositeLifeCycleInInitialRender).toBe( - CompositeComponentLifeCycle.MOUNTING + null ); expect(getLifeCycleState(instance)).toBe(ComponentLifeCycle.MOUNTED);