diff --git a/src/isomorphic/ReactDebugInstanceMap.js b/src/isomorphic/ReactDebugInstanceMap.js index 50dddf4ad0e..9a5236f693e 100644 --- a/src/isomorphic/ReactDebugInstanceMap.js +++ b/src/isomorphic/ReactDebugInstanceMap.js @@ -35,21 +35,12 @@ function checkValidInstance(internalInstance) { return isValid; } -var idCounter = 1; var instancesByIDs = {}; -var instancesToIDs; function getIDForInstance(internalInstance) { - if (!instancesToIDs) { - instancesToIDs = new WeakMap(); - } - if (instancesToIDs.has(internalInstance)) { - return instancesToIDs.get(internalInstance); - } else { - var instanceID = (idCounter++).toString(); - instancesToIDs.set(internalInstance, instanceID); - return instanceID; - } + // This is only available in __DEV__. + // Eventually we might want to use WeakMap for storing IDs. + return internalInstance._debugID; } function getInstanceByID(instanceID) { @@ -86,15 +77,18 @@ var ReactDebugInstanceMap = { } return getIDForInstance(internalInstance); }, + getInstanceByID(instanceID) { return getInstanceByID(instanceID); }, + isRegisteredInstance(internalInstance) { if (!checkValidInstance(internalInstance)) { return false; } return isRegisteredInstance(internalInstance); }, + registerInstance(internalInstance) { if (!checkValidInstance(internalInstance)) { return; @@ -107,6 +101,7 @@ var ReactDebugInstanceMap = { ); registerInstance(internalInstance); }, + unregisterInstance(internalInstance) { if (!checkValidInstance(internalInstance)) { return; diff --git a/src/isomorphic/ReactDebugTool.js b/src/isomorphic/ReactDebugTool.js index 4d2c2a3536a..04c458647e0 100644 --- a/src/isomorphic/ReactDebugTool.js +++ b/src/isomorphic/ReactDebugTool.js @@ -11,6 +11,7 @@ 'use strict'; +var ReactDebugInstanceMap = require('ReactDebugInstanceMap'); var ReactInvalidSetStateWarningDevTool = require('ReactInvalidSetStateWarningDevTool'); var warning = require('warning'); @@ -58,6 +59,10 @@ var ReactDebugTool = { onSetState() { emitEvent('onSetState'); }, + onInstantiateComponent(internalInstance) { + ReactDebugInstanceMap.registerInstance(internalInstance); + emitEvent('onInstantiateComponent', internalInstance); + }, onMountRootComponent(internalInstance) { emitEvent('onMountRootComponent', internalInstance); }, @@ -69,6 +74,7 @@ var ReactDebugTool = { }, onUnmountComponent(internalInstance) { emitEvent('onUnmountComponent', internalInstance); + ReactDebugInstanceMap.unregisterInstance(internalInstance); }, }; diff --git a/src/isomorphic/__tests__/ReactDebugInstanceMap-test.js b/src/isomorphic/__tests__/ReactDebugInstanceMap-test.js index d9a063e2c64..abb20113c22 100644 --- a/src/isomorphic/__tests__/ReactDebugInstanceMap-test.js +++ b/src/isomorphic/__tests__/ReactDebugInstanceMap-test.js @@ -15,16 +15,29 @@ describe('ReactDebugInstanceMap', function() { var React; var ReactDebugInstanceMap; var ReactDOM; + var ReactDOMComponentTree; + var ReactDOMServer; + var ReactInstanceMap; + + var nextDebugID = 0; beforeEach(function() { jest.resetModuleRegistry(); React = require('React'); ReactDebugInstanceMap = require('ReactDebugInstanceMap'); ReactDOM = require('ReactDOM'); + ReactDOMComponentTree = require('ReactDOMComponentTree'); + ReactDOMServer = require('ReactDOMServer'); + ReactInstanceMap = require('ReactInstanceMap'); + + nextDebugID = 0; }); function createStubInstance() { - return { mountComponent: () => {} }; + return { + mountComponent: () => {}, + _debugID: (nextDebugID++).toString(), + }; } it('should register and unregister instances', function() { @@ -170,4 +183,115 @@ describe('ReactDebugInstanceMap', function() { ); } }); + + describe('integration', () => { + describe('ReactDOM', () => { + it('registers native components', () => { + var div = document.createElement('div'); + + var spanInst; + ReactDOM.render( + { + if (span) { + spanInst = ReactDOMComponentTree.getInstanceFromNode(span); + } + }} />, + div + ); + expect(ReactDebugInstanceMap.isRegisteredInstance(spanInst)).toBe(true); + + var pInst; + ReactDOM.render( +

{ + if (p) { + pInst = ReactDOMComponentTree.getInstanceFromNode(p); + } + }} />, + div + ); + expect(ReactDebugInstanceMap.isRegisteredInstance(spanInst)).toBe(false); + expect(ReactDebugInstanceMap.isRegisteredInstance(pInst)).toBe(true); + + ReactDOM.unmountComponentAtNode(div); + expect(ReactDebugInstanceMap.isRegisteredInstance(spanInst)).toBe(false); + expect(ReactDebugInstanceMap.isRegisteredInstance(pInst)).toBe(false); + }); + + it('registers composite components', () => { + var fooInst; + var Foo = React.createClass({ + render() { + fooInst = ReactInstanceMap.get(this); + return null; + }, + }); + var barInst; + var Bar = React.createClass({ + render() { + barInst = ReactInstanceMap.get(this); + return null; + }, + }); + var div = document.createElement('div'); + + ReactDOM.render(, div); + expect(ReactDebugInstanceMap.isRegisteredInstance(fooInst)).toBe(true); + + ReactDOM.render(, div); + expect(ReactDebugInstanceMap.isRegisteredInstance(fooInst)).toBe(false); + expect(ReactDebugInstanceMap.isRegisteredInstance(barInst)).toBe(true); + + ReactDOM.unmountComponentAtNode(div); + expect(ReactDebugInstanceMap.isRegisteredInstance(fooInst)).toBe(false); + expect(ReactDebugInstanceMap.isRegisteredInstance(barInst)).toBe(false); + }); + }); + }); + + describe('ReactDOMServer', () => { + it('registers components but unregisters them in the end', () => { + var fooInst; + var Foo = React.createClass({ + render() { + fooInst = ReactInstanceMap.get(this); + expect(ReactDebugInstanceMap.isRegisteredInstance(fooInst)).toBe(true); + return null; + }, + }); + + ReactDOMServer.renderToString(); + expect(ReactDebugInstanceMap.isRegisteredInstance(fooInst)).toBe(false); + + ReactDOMServer.renderToStaticMarkup(); + expect(ReactDebugInstanceMap.isRegisteredInstance(fooInst)).toBe(false); + }); + + it('can be used together with ReactDOM', () => { + var fooInst; + var Foo = React.createClass({ + render() { + fooInst = ReactInstanceMap.get(this); + expect(ReactDebugInstanceMap.isRegisteredInstance(fooInst)).toBe(true); + return null; + }, + }); + var barInst; + var Bar = React.createClass({ + render() { + barInst = ReactInstanceMap.get(this); + expect(ReactDebugInstanceMap.isRegisteredInstance(barInst)).toBe(true); + return

{ReactDOMServer.renderToString()}
; + }, + }); + + var div = document.createElement('div'); + ReactDOM.render(, div); + expect(ReactDebugInstanceMap.isRegisteredInstance(fooInst)).toBe(false); + expect(ReactDebugInstanceMap.isRegisteredInstance(barInst)).toBe(true); + + ReactDOM.unmountComponentAtNode(div); + expect(ReactDebugInstanceMap.isRegisteredInstance(fooInst)).toBe(false); + expect(ReactDebugInstanceMap.isRegisteredInstance(barInst)).toBe(false); + }); + }); }); diff --git a/src/renderers/dom/client/ReactMount.js b/src/renderers/dom/client/ReactMount.js index 7f6f8cac6ff..a1c420fbf7d 100644 --- a/src/renderers/dom/client/ReactMount.js +++ b/src/renderers/dom/client/ReactMount.js @@ -170,7 +170,11 @@ function batchedMountComponentIntoNode( * @see {ReactMount.unmountComponentAtNode} */ function unmountComponentFromNode(instance, container, safely) { - ReactReconciler.unmountComponent(instance, safely); + var transaction = ReactUpdates.ReactReconcileTransaction.getPooled( + /* useCreateElement */ true + ); + ReactReconciler.unmountComponent(instance, transaction, safely); + ReactUpdates.ReactReconcileTransaction.release(transaction); if (container.nodeType === DOC_NODE_TYPE) { container = container.documentElement; diff --git a/src/renderers/dom/client/ReactReconcileTransaction.js b/src/renderers/dom/client/ReactReconcileTransaction.js index fedd883bf1c..7b54019ec8f 100644 --- a/src/renderers/dom/client/ReactReconcileTransaction.js +++ b/src/renderers/dom/client/ReactReconcileTransaction.js @@ -113,6 +113,7 @@ function ReactReconcileTransaction(useCreateElement) { // `ReactTextComponent` checks it in `mountComponent`.` this.renderToStaticMarkup = false; this.reactMountReady = CallbackQueue.getPooled(null); + this.hasReactMountReady = true; this.useCreateElement = useCreateElement; } diff --git a/src/renderers/dom/server/ReactServerRendering.js b/src/renderers/dom/server/ReactServerRendering.js index 5a5d604b7dd..25c45748750 100644 --- a/src/renderers/dom/server/ReactServerRendering.js +++ b/src/renderers/dom/server/ReactServerRendering.js @@ -14,6 +14,7 @@ var ReactDOMContainerInfo = require('ReactDOMContainerInfo'); var ReactDefaultBatchingStrategy = require('ReactDefaultBatchingStrategy'); var ReactElement = require('ReactElement'); var ReactMarkupChecksum = require('ReactMarkupChecksum'); +var ReactReconciler = require('ReactReconciler'); var ReactServerBatchingStrategy = require('ReactServerBatchingStrategy'); var ReactServerRenderingTransaction = require('ReactServerRenderingTransaction'); @@ -36,12 +37,19 @@ function renderToStringImpl(element, makeStaticMarkup) { return transaction.perform(function() { var componentInstance = instantiateReactComponent(element); - var markup = componentInstance.mountComponent( + var markup = ReactReconciler.mountComponent( + componentInstance, transaction, null, ReactDOMContainerInfo(), emptyObject ); + if (__DEV__) { + // We unmount in __DEV__ to clean up the ReactDebugInstanceMap + // registrations that occur during instance construction. + // We don't do this pass in prod because we don't use it there. + ReactReconciler.unmountComponent(componentInstance, transaction); + } if (!makeStaticMarkup) { markup = ReactMarkupChecksum.addChecksumToMarkup(markup); } diff --git a/src/renderers/dom/server/ReactServerRenderingTransaction.js b/src/renderers/dom/server/ReactServerRenderingTransaction.js index 176a550c599..c56aa86f347 100644 --- a/src/renderers/dom/server/ReactServerRenderingTransaction.js +++ b/src/renderers/dom/server/ReactServerRenderingTransaction.js @@ -34,6 +34,7 @@ function ReactServerRenderingTransaction(renderToStaticMarkup) { this.reinitializeTransaction(); this.renderToStaticMarkup = renderToStaticMarkup; this.useCreateElement = false; + this.hasReactMountReady = false; } var Mixin = { diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js index c897cb30efc..36f23beafaa 100644 --- a/src/renderers/dom/shared/ReactDOMComponent.js +++ b/src/renderers/dom/shared/ReactDOMComponent.js @@ -991,16 +991,16 @@ ReactDOMComponent.Mixin = { if (lastChildren != null && nextChildren == null) { this.updateChildren(null, transaction, context); } else if (lastHasContentOrHtml && !nextHasContentOrHtml) { - this.updateTextContent(''); + this.updateTextContent('', transaction); } if (nextContent != null) { if (lastContent !== nextContent) { - this.updateTextContent('' + nextContent); + this.updateTextContent('' + nextContent, transaction); } } else if (nextHtml != null) { if (lastHtml !== nextHtml) { - this.updateMarkup('' + nextHtml); + this.updateMarkup('' + nextHtml, transaction); } } else if (nextChildren != null) { this.updateChildren(nextChildren, transaction, context); @@ -1017,7 +1017,7 @@ ReactDOMComponent.Mixin = { * * @internal */ - unmountComponent: function(safely) { + unmountComponent: function(transaction, safely) { switch (this._tag) { case 'iframe': case 'object': @@ -1042,7 +1042,7 @@ ReactDOMComponent.Mixin = { * management. So we just document it and throw in dangerous cases. */ invariant( - false, + !transaction.hasReactMountReady, '<%s> tried to unmount. Because of cross-browser quirks it is ' + 'impossible to unmount some top-level components (eg , ' + ', and ) reliably and efficiently. To fix this, have a ' + @@ -1053,7 +1053,7 @@ ReactDOMComponent.Mixin = { break; } - this.unmountChildren(safely); + this.unmountChildren(transaction, safely); ReactDOMComponentTree.uncacheNode(this); EventPluginHub.deleteAllListeners(this); ReactComponentBrowserEnvironment.unmountIDFromEnvironment(this._rootNodeID); diff --git a/src/renderers/shared/reconciler/ReactChildReconciler.js b/src/renderers/shared/reconciler/ReactChildReconciler.js index 634b3444c71..e35f7b2cc29 100644 --- a/src/renderers/shared/reconciler/ReactChildReconciler.js +++ b/src/renderers/shared/reconciler/ReactChildReconciler.js @@ -100,7 +100,7 @@ var ReactChildReconciler = { } else { if (prevChild) { removedNodes[name] = ReactReconciler.getNativeNode(prevChild); - ReactReconciler.unmountComponent(prevChild, false); + ReactReconciler.unmountComponent(prevChild, transaction, false); } // The child must be instantiated before it's mounted. var nextChildInstance = instantiateReactComponent(nextElement); @@ -113,7 +113,7 @@ var ReactChildReconciler = { !(nextChildren && nextChildren.hasOwnProperty(name))) { prevChild = prevChildren[name]; removedNodes[name] = ReactReconciler.getNativeNode(prevChild); - ReactReconciler.unmountComponent(prevChild, false); + ReactReconciler.unmountComponent(prevChild, transaction, false); } } }, @@ -125,11 +125,11 @@ var ReactChildReconciler = { * @param {?object} renderedChildren Previously initialized set of children. * @internal */ - unmountChildren: function(renderedChildren, safely) { + unmountChildren: function(renderedChildren, transaction, safely) { for (var name in renderedChildren) { if (renderedChildren.hasOwnProperty(name)) { var renderedChild = renderedChildren[name]; - ReactReconciler.unmountComponent(renderedChild, safely); + ReactReconciler.unmountComponent(renderedChild, transaction, safely); } } }, diff --git a/src/renderers/shared/reconciler/ReactCompositeComponent.js b/src/renderers/shared/reconciler/ReactCompositeComponent.js index 0472e1fa86e..23ba73edcad 100644 --- a/src/renderers/shared/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/reconciler/ReactCompositeComponent.js @@ -343,7 +343,7 @@ var ReactCompositeComponentMixin = { } checkpoint = transaction.checkpoint(); - this._renderedComponent.unmountComponent(true); + ReactReconciler.unmountComponent(this._renderedComponent, transaction, true); transaction.rollback(checkpoint); // Try again - we've informed the component about the error, so they can render an error message this time. @@ -395,23 +395,25 @@ var ReactCompositeComponentMixin = { * @final * @internal */ - unmountComponent: function(safely) { + unmountComponent: function(transaction, safely) { if (!this._renderedComponent) { return; } var inst = this._instance; - if (inst.componentWillUnmount) { - if (safely) { - var name = this.getName() + '.componentWillUnmount()'; - ReactErrorUtils.invokeGuardedCallback(name, inst.componentWillUnmount.bind(inst)); - } else { - inst.componentWillUnmount(); + if (transaction.hasReactMountReady) { + if (inst.componentWillUnmount) { + if (safely) { + var name = this.getName() + '.componentWillUnmount()'; + ReactErrorUtils.invokeGuardedCallback(name, inst.componentWillUnmount.bind(inst)); + } else { + inst.componentWillUnmount(); + } } } if (this._renderedComponent) { - ReactReconciler.unmountComponent(this._renderedComponent, safely); + ReactReconciler.unmountComponent(this._renderedComponent, transaction, safely); this._renderedNodeType = null; this._renderedComponent = null; this._instance = null; @@ -843,7 +845,7 @@ var ReactCompositeComponentMixin = { ); } else { var oldNativeNode = ReactReconciler.getNativeNode(prevComponentInstance); - ReactReconciler.unmountComponent(prevComponentInstance, false); + ReactReconciler.unmountComponent(prevComponentInstance, transaction, false); this._renderedNodeType = ReactNodeTypes.getType(nextRenderedElement); this._renderedComponent = this._instantiateReactComponent( diff --git a/src/renderers/shared/reconciler/ReactMultiChild.js b/src/renderers/shared/reconciler/ReactMultiChild.js index 66f38613218..65bd1832ac7 100644 --- a/src/renderers/shared/reconciler/ReactMultiChild.js +++ b/src/renderers/shared/reconciler/ReactMultiChild.js @@ -239,10 +239,10 @@ var ReactMultiChild = { * @param {string} nextContent String of content. * @internal */ - updateTextContent: function(nextContent) { + updateTextContent: function(nextContent, transaction) { var prevChildren = this._renderedChildren; // Remove any rendered children. - ReactChildReconciler.unmountChildren(prevChildren, false); + ReactChildReconciler.unmountChildren(prevChildren, transaction, false); for (var name in prevChildren) { if (prevChildren.hasOwnProperty(name)) { invariant(false, 'updateTextContent called on non-empty component.'); @@ -259,10 +259,10 @@ var ReactMultiChild = { * @param {string} nextMarkup String of markup. * @internal */ - updateMarkup: function(nextMarkup) { + updateMarkup: function(nextMarkup, transaction) { var prevChildren = this._renderedChildren; // Remove any rendered children. - ReactChildReconciler.unmountChildren(prevChildren, false); + ReactChildReconciler.unmountChildren(prevChildren, transaction, false); for (var name in prevChildren) { if (prevChildren.hasOwnProperty(name)) { invariant(false, 'updateTextContent called on non-empty component.'); @@ -366,9 +366,9 @@ var ReactMultiChild = { * * @internal */ - unmountChildren: function(safely) { + unmountChildren: function(transaction, safely) { var renderedChildren = this._renderedChildren; - ReactChildReconciler.unmountChildren(renderedChildren, safely); + ReactChildReconciler.unmountChildren(renderedChildren, transaction, safely); this._renderedChildren = null; }, diff --git a/src/renderers/shared/reconciler/ReactReconciler.js b/src/renderers/shared/reconciler/ReactReconciler.js index ae7806e4fc0..6a6b8cfca0e 100644 --- a/src/renderers/shared/reconciler/ReactReconciler.js +++ b/src/renderers/shared/reconciler/ReactReconciler.js @@ -72,9 +72,11 @@ var ReactReconciler = { * @final * @internal */ - unmountComponent: function(internalInstance, safely) { - ReactRef.detachRefs(internalInstance, internalInstance._currentElement); - internalInstance.unmountComponent(safely); + unmountComponent: function(internalInstance, transaction, safely) { + if (transaction.hasReactMountReady) { + ReactRef.detachRefs(internalInstance, internalInstance._currentElement); + } + internalInstance.unmountComponent(transaction, safely); if (__DEV__) { ReactInstrumentation.debugTool.onUnmountComponent(internalInstance); } diff --git a/src/renderers/shared/reconciler/ReactSimpleEmptyComponent.js b/src/renderers/shared/reconciler/ReactSimpleEmptyComponent.js index 0565a016441..e62042cb288 100644 --- a/src/renderers/shared/reconciler/ReactSimpleEmptyComponent.js +++ b/src/renderers/shared/reconciler/ReactSimpleEmptyComponent.js @@ -38,8 +38,8 @@ Object.assign(ReactSimpleEmptyComponent.prototype, { getNativeNode: function() { return ReactReconciler.getNativeNode(this._renderedComponent); }, - unmountComponent: function() { - ReactReconciler.unmountComponent(this._renderedComponent); + unmountComponent: function(transaction) { + ReactReconciler.unmountComponent(this._renderedComponent, transaction); this._renderedComponent = null; }, }); diff --git a/src/renderers/shared/reconciler/instantiateReactComponent.js b/src/renderers/shared/reconciler/instantiateReactComponent.js index 796e037b995..d29873c1c54 100644 --- a/src/renderers/shared/reconciler/instantiateReactComponent.js +++ b/src/renderers/shared/reconciler/instantiateReactComponent.js @@ -13,6 +13,7 @@ var ReactCompositeComponent = require('ReactCompositeComponent'); var ReactEmptyComponent = require('ReactEmptyComponent'); +var ReactInstrumentation = require('ReactInstrumentation'); var ReactNativeComponent = require('ReactNativeComponent'); var invariant = require('invariant'); @@ -56,6 +57,8 @@ function isInternalComponentType(type) { ); } +var nextDebugID = 0; + /** * Given a ReactNode, create an instance that will actually be mounted. * @@ -119,6 +122,7 @@ function instantiateReactComponent(node) { if (__DEV__) { instance._isOwnerNecessary = false; instance._warnedAboutRefsInRender = false; + instance._debugID = (nextDebugID++).toString(); } // Internal instances should fully constructed at this point, so they should @@ -129,6 +133,10 @@ function instantiateReactComponent(node) { } } + if (__DEV__) { + ReactInstrumentation.debugTool.onInstantiateComponent(instance); + } + return instance; } diff --git a/src/test/ReactTestUtils.js b/src/test/ReactTestUtils.js index d1365b50c54..2320cec36d9 100644 --- a/src/test/ReactTestUtils.js +++ b/src/test/ReactTestUtils.js @@ -23,6 +23,7 @@ var ReactElement = require('ReactElement'); var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter'); var ReactCompositeComponent = require('ReactCompositeComponent'); var ReactInstanceMap = require('ReactInstanceMap'); +var ReactInstrumentation = require('ReactInstrumentation'); var ReactUpdates = require('ReactUpdates'); var SyntheticEvent = require('SyntheticEvent'); @@ -369,6 +370,9 @@ ReactShallowRenderer.prototype.getMountedInstance = function() { var NoopInternalComponent = function(element) { this._renderedOutput = element; this._currentElement = element; + if (__DEV__) { + ReactInstrumentation.debugTool.onInstantiateComponent(this); + } }; NoopInternalComponent.prototype = { @@ -455,7 +459,9 @@ ReactShallowRenderer.prototype.getRenderOutput = function() { ReactShallowRenderer.prototype.unmount = function() { if (this._instance) { - this._instance.unmountComponent(false); + var transaction = ReactUpdates.ReactReconcileTransaction.getPooled(true); + this._instance.unmountComponent(transaction, false); + ReactUpdates.ReactReconcileTransaction.release(transaction); } };