Skip to content
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.

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

Consolidate hook events #7472

Merged
merged 8 commits into from
Aug 11, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
26 changes: 14 additions & 12 deletions src/isomorphic/hooks/ReactComponentTreeHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ function remove(id) {
delete itemByKey[key];
}

function create(id, element) {
function create(id, element, parentID) {
var key = getKeyFromID(id);
itemByKey[key] = {
element,
parentID: null,
parentID,
text: null,
childIDs: [],
isMounted: false,
Expand Down Expand Up @@ -133,22 +133,17 @@ var ReactComponentTreeHook = {
}
invariant(
nextChild.parentID === id,
'Expected onSetParent() and onSetChildren() to be consistent (%s ' +
'has parents %s and %s).',
'Expected onBeforeMountComponent() parent and onSetChildren() to ' +
'be consistent (%s has parents %s and %s).',
nextChildID,
nextChild.parentID,
id
);
}
},

onSetParent(id, parentID) {
var item = get(id);
item.parentID = parentID;
},

onInstantiateComponent(id, element) {
create(id, element);
onBeforeMountComponent(id, element, parentID) {
create(id, element, parentID);
},

onBeforeUpdateComponent(id, element) {
Expand Down Expand Up @@ -182,7 +177,14 @@ var ReactComponentTreeHook = {

onUnmountComponent(id) {
var item = get(id);
item.isMounted = false;
if (item) {
// We need to check if it exists.
// `item` might not exist if it is inside an error boundary, and a sibling
// error boundary child threw while mounting. Then this instance never
// got a chance to mount, but it still gets an unmounting event during
// the error boundary cleanup.
item.isMounted = false;
}
unmountedIDs[id] = true;
delete rootIDs[id];
},
Expand Down
7 changes: 6 additions & 1 deletion src/renderers/dom/client/ReactMount.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,17 @@ function mountComponentIntoNode(
console.time(markerName);
}

var parentDebugID;
if (__DEV__) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we drop these __DEV__ blocks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea.

parentDebugID = 0; // top-level component has no parent
}
var markup = ReactReconciler.mountComponent(
wrapperInstance,
transaction,
null,
ReactDOMContainerInfo(wrapperInstance, container),
context
context,
parentDebugID
);

if (markerName) {
Expand Down
7 changes: 6 additions & 1 deletion src/renderers/dom/server/ReactServerRendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,17 @@ function renderToStringImpl(element, makeStaticMarkup) {

return transaction.perform(function() {
var componentInstance = instantiateReactComponent(element, true);
var parentDebugID;
if (__DEV__) {
parentDebugID = 0; // top-level component has no parent
}
var markup = ReactReconciler.mountComponent(
componentInstance,
transaction,
null,
ReactDOMContainerInfo(),
emptyObject
emptyObject,
parentDebugID
);
if (__DEV__) {
ReactInstrumentation.debugTool.onUnmountComponent(
Expand Down
3 changes: 1 addition & 2 deletions src/renderers/dom/shared/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,7 @@ if (__DEV__) {
ReactInstrumentation.debugTool.onBeforeUpdateComponent(contentDebugID, content);
ReactInstrumentation.debugTool.onUpdateComponent(contentDebugID);
} else {
ReactInstrumentation.debugTool.onInstantiateComponent(contentDebugID, content);
ReactInstrumentation.debugTool.onSetParent(contentDebugID, debugID);
ReactInstrumentation.debugTool.onBeforeMountComponent(contentDebugID, content, debugID);
ReactInstrumentation.debugTool.onMountComponent(contentDebugID);
ReactInstrumentation.debugTool.onSetChildren(debugID, [contentDebugID]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function handleElement(debugID, element) {
}

var ReactDOMNullInputValuePropHook = {
onInstantiateComponent(debugID, element) {
onBeforeMountComponent(debugID, element) {
handleElement(debugID, element);
},
onBeforeUpdateComponent(debugID, element) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ function handleElement(debugID, element) {
}

var ReactDOMUnknownPropertyHook = {
onInstantiateComponent(debugID, element) {
onBeforeMountComponent(debugID, element) {
handleElement(debugID, element);
},
onBeforeUpdateComponent(debugID, element) {
Expand Down
7 changes: 6 additions & 1 deletion src/renderers/native/ReactNativeMount.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,17 @@ function mountComponentIntoNode(
componentInstance,
containerTag,
transaction) {
var parentDebugID;
if (__DEV__) {
parentDebugID = 0; // top-level component has no parent
}
var markup = ReactReconciler.mountComponent(
componentInstance,
transaction,
null,
ReactNativeContainerInfo(containerTag),
emptyObject
emptyObject,
parentDebugID
);
componentInstance._renderedComponent._topLevelWrapper = componentInstance;
ReactNativeMount._mountImageIntoNode(markup, containerTag);
Expand Down
18 changes: 7 additions & 11 deletions src/renderers/shared/ReactDebugTool.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ function resetMeasurements() {
currentFlushMeasurements = [];
}

function checkDebugID(debugID) {
function checkDebugID(debugID, allowRoot = false) {
if (allowRoot && debugID === 0) {
return;
}
if (!debugID) {
warning(false, 'ReactDebugTool: debugID may not be empty.');
}
Expand Down Expand Up @@ -272,21 +275,14 @@ var ReactDebugTool = {
childDebugIDs.forEach(checkDebugID);
emitEvent('onSetChildren', debugID, childDebugIDs);
},
onSetParent(debugID, parentDebugID) {
checkDebugID(debugID);
emitEvent('onSetParent', debugID, parentDebugID);
},
onInstantiateComponent(debugID, element) {
checkDebugID(debugID);
emitEvent('onInstantiateComponent', debugID, element);
},
onMountRootComponent(debugID) {
checkDebugID(debugID);
emitEvent('onMountRootComponent', debugID);
},
onBeforeMountComponent(debugID) {
onBeforeMountComponent(debugID, element, parentDebugID) {
checkDebugID(debugID);
emitEvent('onBeforeMountComponent', debugID);
checkDebugID(parentDebugID, true);
emitEvent('onBeforeMountComponent', debugID, element, parentDebugID);
},
onMountComponent(debugID) {
checkDebugID(debugID);
Expand Down
7 changes: 5 additions & 2 deletions src/renderers/shared/stack/reconciler/ReactChildReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ var ReactChildReconciler = {
transaction,
hostParent,
hostContainerInfo,
context) {
context,
selfDebugID // __DEV__ only
) {
// We currently don't have a way to track moves here but if we use iterators
// instead of for..in we can zip the iterators and check if an item has
// moved.
Expand Down Expand Up @@ -156,7 +158,8 @@ var ReactChildReconciler = {
transaction,
hostParent,
hostContainerInfo,
context
context,
selfDebugID
);
mountImages.push(nextChildMountImage);
}
Expand Down
26 changes: 10 additions & 16 deletions src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -525,21 +525,18 @@ var ReactCompositeComponentMixin = {
nodeType !== ReactNodeTypes.EMPTY /* shouldHaveDebugID */
);
this._renderedComponent = child;

var selfDebugID;
if (__DEV__) {
if (child._debugID !== 0 && this._debugID !== 0) {
ReactInstrumentation.debugTool.onSetParent(
child._debugID,
this._debugID
);
}
selfDebugID = this._debugID;
}

var markup = ReactReconciler.mountComponent(
child,
transaction,
hostParent,
hostContainerInfo,
this._processChildContext(context)
this._processChildContext(context),
selfDebugID
);

if (__DEV__) {
Expand Down Expand Up @@ -1047,21 +1044,18 @@ var ReactCompositeComponentMixin = {
nodeType !== ReactNodeTypes.EMPTY /* shouldHaveDebugID */
);
this._renderedComponent = child;

var selfDebugID;
if (__DEV__) {
if (child._debugID !== 0 && this._debugID !== 0) {
ReactInstrumentation.debugTool.onSetParent(
child._debugID,
this._debugID
);
}
selfDebugID = this._debugID;
}

var nextMarkup = ReactReconciler.mountComponent(
child,
transaction,
this._hostParent,
this._hostContainerInfo,
this._processChildContext(context)
this._processChildContext(context),
selfDebugID
);

if (__DEV__) {
Expand Down
31 changes: 15 additions & 16 deletions src/renderers/shared/stack/reconciler/ReactMultiChild.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ function processQueue(inst, updateQueue) {
);
}

var setParentForInstrumentation = emptyFunction;
var setChildrenForInstrumentation = emptyFunction;
if (__DEV__) {
var getDebugID = function(inst) {
Expand All @@ -153,14 +152,6 @@ if (__DEV__) {
}
return inst._debugID;
};
setParentForInstrumentation = function(child) {
if (child._debugID !== 0) {
ReactInstrumentation.debugTool.onSetParent(
child._debugID,
getDebugID(this)
);
}
};
setChildrenForInstrumentation = function(children) {
var debugID = getDebugID(this);
// TODO: React Native empty components are also multichild.
Expand Down Expand Up @@ -193,11 +184,12 @@ var ReactMultiChild = {

_reconcilerInstantiateChildren: function(nestedChildren, transaction, context) {
if (__DEV__) {
var selfDebugID = getDebugID(this);
if (this._currentElement) {
try {
ReactCurrentOwner.current = this._currentElement._owner;
return ReactChildReconciler.instantiateChildren(
nestedChildren, transaction, context, this._debugID
nestedChildren, transaction, context, selfDebugID
);
} finally {
ReactCurrentOwner.current = null;
Expand All @@ -218,11 +210,14 @@ var ReactMultiChild = {
context
) {
var nextChildren;
var selfDebugID;

if (__DEV__) {
selfDebugID = getDebugID(this);
if (this._currentElement) {
try {
ReactCurrentOwner.current = this._currentElement._owner;
nextChildren = flattenChildren(nextNestedChildrenElements, this._debugID);
nextChildren = flattenChildren(nextNestedChildrenElements, selfDebugID);
} finally {
ReactCurrentOwner.current = null;
}
Expand All @@ -234,12 +229,13 @@ var ReactMultiChild = {
transaction,
this,
this._hostContainerInfo,
context
context,
selfDebugID
);
return nextChildren;
}
}
nextChildren = flattenChildren(nextNestedChildrenElements);
nextChildren = flattenChildren(nextNestedChildrenElements, selfDebugID);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interesting that we weren’t passing it before (this code path could still run in __DEV__).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, when is this._currentElement not set? Not even sure why we have that check.

ReactChildReconciler.updateChildren(
prevChildren,
nextChildren,
Expand All @@ -248,7 +244,8 @@ var ReactMultiChild = {
transaction,
this,
this._hostContainerInfo,
context
context,
selfDebugID
);
return nextChildren;
},
Expand All @@ -272,15 +269,17 @@ var ReactMultiChild = {
for (var name in children) {
if (children.hasOwnProperty(name)) {
var child = children[name];
var selfDebugID;
if (__DEV__) {
setParentForInstrumentation.call(this, child);
selfDebugID = getDebugID(this);
}
var mountImage = ReactReconciler.mountComponent(
child,
transaction,
this,
this._hostContainerInfo,
context
context,
selfDebugID
);
child._mountIndex = index++;
mountImages.push(mountImage);
Expand Down
10 changes: 7 additions & 3 deletions src/renderers/shared/stack/reconciler/ReactReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,24 @@ var ReactReconciler = {
transaction,
hostParent,
hostContainerInfo,
context
context,
parentDebugID // __DEV__ only
) {
if (__DEV__) {
if (internalInstance._debugID !== 0) {
ReactInstrumentation.debugTool.onBeforeMountComponent(
internalInstance._debugID
internalInstance._debugID,
internalInstance._currentElement,
parentDebugID
);
}
}
var markup = internalInstance.mountComponent(
transaction,
hostParent,
hostContainerInfo,
context
context,
parentDebugID
);
if (internalInstance._currentElement &&
internalInstance._currentElement.ref != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,16 @@ Object.assign(ReactSimpleEmptyComponent.prototype, {
transaction,
hostParent,
hostContainerInfo,
context
context,
parentDebugID // __DEV__ only
) {
return ReactReconciler.mountComponent(
this._renderedComponent,
transaction,
hostParent,
hostContainerInfo,
context
context,
parentDebugID
);
},
receiveComponent: function() {
Expand Down
Loading