Skip to content

Track mounted instances with ReactDebugInstanceMap #6486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gaearon wants to merge 2 commits into from
Closed

Track mounted instances with ReactDebugInstanceMap #6486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ad0aa4
Track mounted instances with ReactDebugInstanceMap
gaearon Apr 11, 2016
f99fc80
Replace WeakMap with a dev-only _debugID instance field
gaearon Apr 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions src/isomorphic/ReactDebugInstanceMap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -107,6 +101,7 @@ var ReactDebugInstanceMap = {
);
registerInstance(internalInstance);
},

unregisterInstance(internalInstance) {
if (!checkValidInstance(internalInstance)) {
return;
Expand Down
6 changes: 6 additions & 0 deletions src/isomorphic/ReactDebugTool.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

'use strict';

var ReactDebugInstanceMap = require('ReactDebugInstanceMap');
var ReactInvalidSetStateWarningDevTool = require('ReactInvalidSetStateWarningDevTool');
var warning = require('warning');

Expand Down Expand Up @@ -58,6 +59,10 @@ var ReactDebugTool = {
onSetState() {
emitEvent('onSetState');
},
onInstantiateComponent(internalInstance) {
ReactDebugInstanceMap.registerInstance(internalInstance);
Copy link
Collaborator

@sebmarkbage sebmarkbage Apr 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remind me again why this needs to be here and not in a DevTool that listens to the event below?

If it was attached conditionally by a devtool, then we wouldn't take on a dependency on WeakMap. Only in environments that needs that tool.

We definitely can't take on a dependency on WeakMap for all environments. Would make it impossible debug many IE versions.

Copy link
Collaborator Author

@gaearon gaearon Apr 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to make sure this happens before any devtool receives the event. (Since devtools are actually going to get IDs—I will implement this separately later but #6046 shows how it could work).

I also want to have the opposite guarantee for unmounting: that ID gets unregistered after all the other devtools have run. This is why putting such a devtool as first in list wouldn’t work.

emitEvent('onInstantiateComponent', internalInstance);
},
onMountRootComponent(internalInstance) {
emitEvent('onMountRootComponent', internalInstance);
},
Expand All @@ -69,6 +74,7 @@ var ReactDebugTool = {
},
onUnmountComponent(internalInstance) {
emitEvent('onUnmountComponent', internalInstance);
ReactDebugInstanceMap.unregisterInstance(internalInstance);
},
};

Expand Down
126 changes: 125 additions & 1 deletion src/isomorphic/__tests__/ReactDebugInstanceMap-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -170,4 +183,115 @@ describe('ReactDebugInstanceMap', function() {
);
}
});

describe('integration', () => {
describe('ReactDOM', () => {
it('registers native components', () => {
var div = document.createElement('div');

var spanInst;
ReactDOM.render(
<span ref={span => {
if (span) {
spanInst = ReactDOMComponentTree.getInstanceFromNode(span);
}
}} />,
div
);
expect(ReactDebugInstanceMap.isRegisteredInstance(spanInst)).toBe(true);

var pInst;
ReactDOM.render(
<p ref={p => {
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(<Foo />, div);
expect(ReactDebugInstanceMap.isRegisteredInstance(fooInst)).toBe(true);

ReactDOM.render(<Bar />, 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(<Foo />);
expect(ReactDebugInstanceMap.isRegisteredInstance(fooInst)).toBe(false);

ReactDOMServer.renderToStaticMarkup(<Foo />);
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 <div>{ReactDOMServer.renderToString(<Foo />)}</div>;
},
});

var div = document.createElement('div');
ReactDOM.render(<Bar />, 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);
});
});
});
6 changes: 5 additions & 1 deletion src/renderers/dom/client/ReactMount.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/renderers/dom/client/ReactReconcileTransaction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
10 changes: 9 additions & 1 deletion src/renderers/dom/server/ReactServerRendering.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions src/renderers/dom/server/ReactServerRenderingTransaction.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ function ReactServerRenderingTransaction(renderToStaticMarkup) {
this.reinitializeTransaction();
this.renderToStaticMarkup = renderToStaticMarkup;
this.useCreateElement = false;
this.hasReactMountReady = false;
}

var Mixin = {
Expand Down
12 changes: 6 additions & 6 deletions src/renderers/dom/shared/ReactDOMComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -1017,7 +1017,7 @@ ReactDOMComponent.Mixin = {
*
* @internal
*/
unmountComponent: function(safely) {
unmountComponent: function(transaction, safely) {
switch (this._tag) {
case 'iframe':
case 'object':
Expand All @@ -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 <html>, ' +
'<head>, and <body>) reliably and efficiently. To fix this, have a ' +
Expand All @@ -1053,7 +1053,7 @@ ReactDOMComponent.Mixin = {
break;
}

this.unmountChildren(safely);
this.unmountChildren(transaction, safely);
ReactDOMComponentTree.uncacheNode(this);
EventPluginHub.deleteAllListeners(this);
ReactComponentBrowserEnvironment.unmountIDFromEnvironment(this._rootNodeID);
Expand Down
8 changes: 4 additions & 4 deletions src/renderers/shared/reconciler/ReactChildReconciler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
}
},
Expand All @@ -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);
}
}
},
Expand Down
Loading