From 52c7625cf891e72a1ccac9b9a56d93bc0007fc25 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 3 Jul 2017 23:45:56 -0700 Subject: [PATCH 1/5] Extract the top element frame from ReactDebugCurrentFrame This is part of a larger refactor to decouple stack addendums. All renderers have their own way of getting the stack of the currently executing components. There is one special case in Element Validator that adds an additional line for the element being validated. This commit moves that special case in into the validator. There is another case where it looked like this was used in shallow renderer but this is actually something different. It is part of the component stack. It just happens to be that shallow renderer has a simpler implementation of the component stack that just happens to be a single element. This will let us decouple the implementation to get a stack from ReactDebugCurrentFrame and put that in each renderer. --- .../classic/element/ReactDebugCurrentFrame.js | 8 +-- .../classic/element/ReactElementValidator.js | 64 +++++++++++-------- .../hooks/ReactComponentTreeHook.js | 15 +---- .../__tests__/ReactComponentTreeHook-test.js | 24 +++---- .../testing/ReactShallowRendererEntry.js | 39 +++++++++-- src/shared/ReactFiberComponentTreeHook.js | 16 +---- src/shared/describeComponentFrame.js | 31 +++++++++ 7 files changed, 121 insertions(+), 76 deletions(-) create mode 100644 src/shared/describeComponentFrame.js diff --git a/src/isomorphic/classic/element/ReactDebugCurrentFrame.js b/src/isomorphic/classic/element/ReactDebugCurrentFrame.js index 38dcf3c73d59d..99d539abef1d9 100644 --- a/src/isomorphic/classic/element/ReactDebugCurrentFrame.js +++ b/src/isomorphic/classic/element/ReactDebugCurrentFrame.js @@ -29,13 +29,9 @@ if (__DEV__) { // Component that is being worked on ReactDebugCurrentFrame.current = (null: Fiber | DebugID | null); - // Element that is being cloned or created - ReactDebugCurrentFrame.element = (null: *); - ReactDebugCurrentFrame.getStackAddendum = function(): string | null { let stack = null; const current = ReactDebugCurrentFrame.current; - const element = ReactDebugCurrentFrame.element; if (current !== null) { if (typeof current === 'number') { // DebugID from Stack. @@ -48,8 +44,8 @@ if (__DEV__) { const workInProgress = current; stack = getStackAddendumByWorkInProgressFiber(workInProgress); } - } else if (element !== null) { - stack = getCurrentStackAddendum(element); + } else { + stack = getCurrentStackAddendum(); } return stack; }; diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js index 6fe05d889e5f6..8aa8b240b4ec5 100644 --- a/src/isomorphic/classic/element/ReactElementValidator.js +++ b/src/isomorphic/classic/element/ReactElementValidator.js @@ -21,14 +21,42 @@ var ReactCurrentOwner = require('ReactCurrentOwner'); var ReactElement = require('ReactElement'); -var getComponentName = require('getComponentName'); - if (__DEV__) { var checkPropTypes = require('prop-types/checkPropTypes'); var lowPriorityWarning = require('lowPriorityWarning'); var ReactDebugCurrentFrame = require('ReactDebugCurrentFrame'); var warning = require('fbjs/lib/warning'); - var {getCurrentStackAddendum} = require('ReactComponentTreeHook'); + var describeComponentFrame = require('describeComponentFrame'); + var getComponentName = require('getComponentName'); + + var currentlyValidatingElement = null; + + var getDisplayName = function(element: ?ReactElement): string { + if (element == null) { + return '#empty'; + } else if (typeof element === 'string' || typeof element === 'number') { + return '#text'; + } else if (typeof element.type === 'string') { + return element.type; + } else { + return element.type.displayName || element.type.name || 'Unknown'; + } + }; + + var getStackAddendum = function(): string { + var stack = ''; + if (currentlyValidatingElement) { + var name = getDisplayName(currentlyValidatingElement); + var owner = currentlyValidatingElement._owner; + stack += describeComponentFrame( + name, + currentlyValidatingElement._source, + owner && getComponentName(owner), + ); + } + stack += ReactDebugCurrentFrame.getStackAddendum() || ''; + return stack; + }; } var ITERATOR_SYMBOL = typeof Symbol === 'function' && Symbol.iterator; @@ -115,14 +143,16 @@ function validateExplicitKey(element, parentType) { childOwner = ` It was passed a child from ${getComponentName(element._owner)}.`; } + currentlyValidatingElement = element; warning( false, 'Each child in an array or iterator should have a unique "key" prop.' + '%s%s See https://fb.me/react-warning-keys for more information.%s', currentComponentErrorInfo, childOwner, - getCurrentStackAddendum(element), + getStackAddendum(), ); + currentlyValidatingElement = null; } /** @@ -193,13 +223,9 @@ function validatePropTypes(element) { : componentClass.propTypes; if (propTypes) { - checkPropTypes( - propTypes, - element.props, - 'prop', - name, - ReactDebugCurrentFrame.getStackAddendum, - ); + currentlyValidatingElement = element; + checkPropTypes(propTypes, element.props, 'prop', name, getStackAddendum); + currentlyValidatingElement = null; } if (typeof componentClass.getDefaultProps === 'function') { warning( @@ -235,7 +261,7 @@ var ReactElementValidator = { info += getDeclarationErrorAddendum(); } - info += getCurrentStackAddendum(); + info += ReactDebugCurrentFrame.getStackAddendum() || ''; warning( false, @@ -255,10 +281,6 @@ var ReactElementValidator = { return element; } - if (__DEV__) { - ReactDebugCurrentFrame.element = element; - } - // Skip key warning if the type isn't valid since our key validation logic // doesn't expect a non-string/function type and can throw confusing errors. // We don't want exception behavior to differ between dev and prod. @@ -272,10 +294,6 @@ var ReactElementValidator = { validatePropTypes(element); - if (__DEV__) { - ReactDebugCurrentFrame.element = null; - } - return element; }, @@ -306,16 +324,10 @@ var ReactElementValidator = { cloneElement: function(element, props, children) { var newElement = ReactElement.cloneElement.apply(this, arguments); - if (__DEV__) { - ReactDebugCurrentFrame.element = newElement; - } for (var i = 2; i < arguments.length; i++) { validateChildKeys(arguments[i], newElement.type); } validatePropTypes(newElement); - if (__DEV__) { - ReactDebugCurrentFrame.element = null; - } return newElement; }, }; diff --git a/src/isomorphic/hooks/ReactComponentTreeHook.js b/src/isomorphic/hooks/ReactComponentTreeHook.js index 41799f7cb6f99..b86a10dda5aef 100644 --- a/src/isomorphic/hooks/ReactComponentTreeHook.js +++ b/src/isomorphic/hooks/ReactComponentTreeHook.js @@ -15,15 +15,14 @@ var ReactCurrentOwner = require('ReactCurrentOwner'); var { getStackAddendumByWorkInProgressFiber, - describeComponentFrame, } = require('ReactFiberComponentTreeHook'); var invariant = require('fbjs/lib/invariant'); var warning = require('fbjs/lib/warning'); +var describeComponentFrame = require('describeComponentFrame'); var getComponentName = require('getComponentName'); import type {ReactElement, Source} from 'ReactElementType'; import type {DebugID} from 'ReactInstanceType'; -import type {Fiber} from 'ReactFiber'; function isNative(fn) { // Based on isNative() from Lodash @@ -313,18 +312,8 @@ var ReactComponentTreeHook = { return item ? item.isMounted : false; }, - getCurrentStackAddendum(topElement: ?ReactElement): string { + getCurrentStackAddendum(): string { var info = ''; - if (topElement) { - var name = getDisplayName(topElement); - var owner = topElement._owner; - info += describeComponentFrame( - name, - topElement._source, - owner && getComponentName(owner), - ); - } - var currentOwner = ReactCurrentOwner.current; if (currentOwner) { if (typeof currentOwner.tag === 'number') { diff --git a/src/renderers/__tests__/ReactComponentTreeHook-test.js b/src/renderers/__tests__/ReactComponentTreeHook-test.js index 5f0a5c904d166..8616591a45945 100644 --- a/src/renderers/__tests__/ReactComponentTreeHook-test.js +++ b/src/renderers/__tests__/ReactComponentTreeHook-test.js @@ -37,7 +37,7 @@ describe('ReactComponentTreeHook', () => { describe('stack addenda', () => { it('gets created', () => { function getAddendum(element) { - var addendum = ReactComponentTreeHook.getCurrentStackAddendum(element); + var addendum = ReactComponentTreeHook.getCurrentStackAddendum(); return addendum.replace(/\(at .+?:\d+\)/g, '(at **)'); } @@ -52,12 +52,12 @@ describe('ReactComponentTreeHook', () => { } expectDev(getAddendum()).toBe(''); - expectDev(getAddendum(
)).toBe('\n in div (at **)'); - expectDev(getAddendum()).toBe('\n in Unknown (at **)'); - expectDev(getAddendum()).toBe('\n in Orange (at **)'); - expectDev(getAddendum(React.createElement(Orange))).toBe( - '\n in Orange', - ); + // expectDev(getAddendum(
)).toBe('\n in div (at **)'); + // expectDev(getAddendum()).toBe('\n in Unknown (at **)'); + // expectDev(getAddendum()).toBe('\n in Orange (at **)'); + // expectDev(getAddendum(React.createElement(Orange))).toBe( + // '\n in Orange', + // ); var renders = 0; var rOwnedByQ; @@ -82,15 +82,15 @@ describe('ReactComponentTreeHook', () => { '\n in Q (at **)', ); expectDev(getAddendum()).toBe( - '\n in span (at **)' + - '\n in S (at **)' + + // '\n in span (at **)' + + '\n in S (at **)' + '\n in div (at **)' + '\n in R (created by Q)' + '\n in Q (at **)', ); expectDev(getAddendum(React.createElement('span'))).toBe( - '\n in span (created by S)' + - '\n in S (at **)' + + // '\n in span (created by S)' + + '\n in S (at **)' + '\n in div (at **)' + '\n in R (created by Q)' + '\n in Q (at **)', @@ -103,7 +103,7 @@ describe('ReactComponentTreeHook', () => { expectDev(renders).toBe(2); // Make sure owner is fetched for the top element too. - expectDev(getAddendum(rOwnedByQ)).toBe('\n in R (created by Q)'); + // expectDev(getAddendum(rOwnedByQ)).toBe('\n in R (created by Q)'); }); // These are features and regression tests that only affect diff --git a/src/renderers/testing/ReactShallowRendererEntry.js b/src/renderers/testing/ReactShallowRendererEntry.js index ac610f1caf056..99682cf3414a0 100644 --- a/src/renderers/testing/ReactShallowRendererEntry.js +++ b/src/renderers/testing/ReactShallowRendererEntry.js @@ -18,7 +18,38 @@ const React = require('react'); const emptyObject = require('fbjs/lib/emptyObject'); const invariant = require('fbjs/lib/invariant'); -const {ReactDebugCurrentFrame} = require('ReactGlobalSharedState'); +if (__DEV__) { + var describeComponentFrame = require('describeComponentFrame'); + var getComponentName = require('getComponentName'); + + var currentlyValidatingElement = null; + + var getDisplayName = function(element: ?ReactElement): string { + if (element == null) { + return '#empty'; + } else if (typeof element === 'string' || typeof element === 'number') { + return '#text'; + } else if (typeof element.type === 'string') { + return element.type; + } else { + return element.type.displayName || element.type.name || 'Unknown'; + } + }; + + var getStackAddendum = function(): string { + var stack = ''; + if (currentlyValidatingElement) { + var name = getDisplayName(currentlyValidatingElement); + var owner = currentlyValidatingElement._owner; + stack += describeComponentFrame( + name, + currentlyValidatingElement._source, + owner && getComponentName(owner), + ); + } + return stack; + }; +} class ReactShallowRenderer { static createRenderer = function() { @@ -79,17 +110,17 @@ class ReactShallowRenderer { ); if (element.type.hasOwnProperty('contextTypes')) { - ReactDebugCurrentFrame.element = element; + currentlyValidatingElement = element; checkPropTypes( element.type.contextTypes, context, 'context', getName(element.type, this._instance), - ReactDebugCurrentFrame.getStackAddendum, + getStackAddendum, ); - ReactDebugCurrentFrame.element = null; + currentlyValidatingElement = null; } this._mountClassComponent(element.props, context); diff --git a/src/shared/ReactFiberComponentTreeHook.js b/src/shared/ReactFiberComponentTreeHook.js index d965c718add79..5f2163f6f0ae9 100644 --- a/src/shared/ReactFiberComponentTreeHook.js +++ b/src/shared/ReactFiberComponentTreeHook.js @@ -19,24 +19,11 @@ var { ClassComponent, HostComponent, } = ReactTypeOfWork; +var describeComponentFrame = require('describeComponentFrame'); var getComponentName = require('getComponentName'); import type {Fiber} from 'ReactFiber'; -function describeComponentFrame(name, source: any, ownerName) { - return ( - '\n in ' + - (name || 'Unknown') + - (source - ? ' (at ' + - source.fileName.replace(/^.*[\\\/]/, '') + - ':' + - source.lineNumber + - ')' - : ownerName ? ' (created by ' + ownerName + ')' : '') - ); -} - function describeFiber(fiber: Fiber): string { switch (fiber.tag) { case IndeterminateComponent: @@ -72,5 +59,4 @@ function getStackAddendumByWorkInProgressFiber(workInProgress: Fiber): string { module.exports = { getStackAddendumByWorkInProgressFiber, - describeComponentFrame, }; diff --git a/src/shared/describeComponentFrame.js b/src/shared/describeComponentFrame.js new file mode 100644 index 0000000000000..67e8fbe3b92eb --- /dev/null +++ b/src/shared/describeComponentFrame.js @@ -0,0 +1,31 @@ +/** + * Copyright 2016-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. + * + * @flow + * @providesModule describeComponentFrame + */ + +'use strict'; + +module.exports = function( + name: null | string, + source: any, + ownerName: null | string, +) { + return ( + '\n in ' + + (name || 'Unknown') + + (source + ? ' (at ' + + source.fileName.replace(/^.*[\\\/]/, '') + + ':' + + source.lineNumber + + ')' + : ownerName ? ' (created by ' + ownerName + ')' : '') + ); +}; From 71276255467389d4d706e72378bc280bf7f2e2a9 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 4 Jul 2017 00:54:05 -0700 Subject: [PATCH 2/5] Stop using ReactComponentTreeHook for Fiber Currently we fall back to ReactCurrentOwner in ReactComponentTreeHook for stack addendums. We shouldn't need to because we should use ReactDebugCurrrentFiber. Ensure we always set both ReactDebugCurrentFiber and ReactDebugCurrentFrame so that we can rely on these for all stacks. --- src/isomorphic/children/traverseAllChildren.js | 6 +++--- src/isomorphic/hooks/ReactComponentTreeHook.js | 15 +++++---------- .../__tests__/ReactComponentTreeHook-test.js | 16 ++++++++++------ .../shared/fiber/ReactDebugCurrentFiber.js | 17 ++++++++++++++++- .../shared/fiber/ReactFiberBeginWork.js | 10 +++++----- .../shared/fiber/ReactFiberCompleteWork.js | 2 +- src/renderers/shared/fiber/ReactFiberContext.js | 17 ++++++++--------- .../shared/fiber/ReactFiberScheduler.js | 11 +++++------ 8 files changed, 53 insertions(+), 41 deletions(-) diff --git a/src/isomorphic/children/traverseAllChildren.js b/src/isomorphic/children/traverseAllChildren.js index 5aa0acf31aa48..3b763c8ce1e67 100644 --- a/src/isomorphic/children/traverseAllChildren.js +++ b/src/isomorphic/children/traverseAllChildren.js @@ -24,7 +24,7 @@ var REACT_ELEMENT_TYPE = 0xeac7; if (__DEV__) { - var {getCurrentStackAddendum} = require('ReactComponentTreeHook'); + var {getStackAddendum} = require('ReactDebugCurrentFrame'); } var SEPARATOR = '.'; @@ -171,7 +171,7 @@ function traverseAllChildrenImpl( 'Using Maps as children is unsupported and will likely yield ' + 'unexpected results. Convert it to a sequence/iterable of keyed ' + 'ReactElements instead.%s', - getCurrentStackAddendum(), + getStackAddendum(), ); didWarnAboutMaps = true; } @@ -196,7 +196,7 @@ function traverseAllChildrenImpl( addendum = ' If you meant to render a collection of children, use an array ' + 'instead.' + - getCurrentStackAddendum(); + getStackAddendum(); } var childrenString = '' + children; invariant( diff --git a/src/isomorphic/hooks/ReactComponentTreeHook.js b/src/isomorphic/hooks/ReactComponentTreeHook.js index b86a10dda5aef..731d335bfa0c6 100644 --- a/src/isomorphic/hooks/ReactComponentTreeHook.js +++ b/src/isomorphic/hooks/ReactComponentTreeHook.js @@ -13,13 +13,9 @@ 'use strict'; var ReactCurrentOwner = require('ReactCurrentOwner'); -var { - getStackAddendumByWorkInProgressFiber, -} = require('ReactFiberComponentTreeHook'); var invariant = require('fbjs/lib/invariant'); var warning = require('fbjs/lib/warning'); var describeComponentFrame = require('describeComponentFrame'); -var getComponentName = require('getComponentName'); import type {ReactElement, Source} from 'ReactElementType'; import type {DebugID} from 'ReactInstanceType'; @@ -316,12 +312,11 @@ var ReactComponentTreeHook = { var info = ''; var currentOwner = ReactCurrentOwner.current; if (currentOwner) { - if (typeof currentOwner.tag === 'number') { - const workInProgress = ((currentOwner: any): Fiber); - // Safe because if current owner exists, we are reconciling, - // and it is guaranteed to be the work-in-progress version. - info += getStackAddendumByWorkInProgressFiber(workInProgress); - } else if (typeof currentOwner._debugID === 'number') { + invariant( + typeof currentOwner.tag !== 'number', + 'Fiber owners should not show up in Stack stack traces.', + ); + if (typeof currentOwner._debugID === 'number') { info += ReactComponentTreeHook.getStackAddendumByID( currentOwner._debugID, ); diff --git a/src/renderers/__tests__/ReactComponentTreeHook-test.js b/src/renderers/__tests__/ReactComponentTreeHook-test.js index 8616591a45945..c7ca3644ecd8c 100644 --- a/src/renderers/__tests__/ReactComponentTreeHook-test.js +++ b/src/renderers/__tests__/ReactComponentTreeHook-test.js @@ -20,6 +20,7 @@ describe('ReactComponentTreeHook', () => { var ReactDOMServer; var ReactInstanceMap; var ReactComponentTreeHook; + var ReactDebugCurrentFiber; var ReactComponentTreeTestUtils; beforeEach(() => { @@ -29,6 +30,7 @@ describe('ReactComponentTreeHook', () => { ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); ReactInstanceMap = require('ReactInstanceMap'); + ReactDebugCurrentFiber = require('ReactDebugCurrentFiber'); ReactComponentTreeHook = require('ReactComponentTreeHook'); ReactComponentTreeTestUtils = require('ReactComponentTreeTestUtils'); }); @@ -37,7 +39,9 @@ describe('ReactComponentTreeHook', () => { describe('stack addenda', () => { it('gets created', () => { function getAddendum(element) { - var addendum = ReactComponentTreeHook.getCurrentStackAddendum(); + var addendum = ReactDOMFeatureFlags.useFiber + ? ReactDebugCurrentFiber.getCurrentFiberStackAddendum() || '' + : ReactComponentTreeHook.getCurrentStackAddendum(); return addendum.replace(/\(at .+?:\d+\)/g, '(at **)'); } @@ -47,9 +51,9 @@ describe('ReactComponentTreeHook', () => { Object.defineProperty(Anon, 'name', { value: null, }); - function Orange() { - return null; - } + // function Orange() { + // return null; + // } expectDev(getAddendum()).toBe(''); // expectDev(getAddendum(
)).toBe('\n in div (at **)'); @@ -60,10 +64,10 @@ describe('ReactComponentTreeHook', () => { // ); var renders = 0; - var rOwnedByQ; + //var rOwnedByQ; function Q() { - return (rOwnedByQ = React.createElement(R)); + return /*rOwnedByQ =*/ React.createElement(R); } function R() { return
; diff --git a/src/renderers/shared/fiber/ReactDebugCurrentFiber.js b/src/renderers/shared/fiber/ReactDebugCurrentFiber.js index 8e6f9c727c66b..4685a8c0d6d40 100644 --- a/src/renderers/shared/fiber/ReactDebugCurrentFiber.js +++ b/src/renderers/shared/fiber/ReactDebugCurrentFiber.js @@ -16,6 +16,8 @@ import type {Fiber} from 'ReactFiber'; type LifeCyclePhase = 'render' | 'getChildContext'; +var {ReactDebugCurrentFrame} = require('ReactGlobalSharedState'); + if (__DEV__) { var getComponentName = require('getComponentName'); var { @@ -49,10 +51,23 @@ function getCurrentFiberStackAddendum(): string | null { return null; } +function resetCurrentFiber() { + ReactDebugCurrentFrame.current = null; + ReactDebugCurrentFiber.current = null; + ReactDebugCurrentFiber.phase = null; +} + +function setCurrentFiber(fiber: Fiber | null, phase: LifeCyclePhase | null) { + ReactDebugCurrentFrame.current = fiber; + ReactDebugCurrentFiber.current = fiber; + ReactDebugCurrentFiber.phase = phase; +} + var ReactDebugCurrentFiber = { current: (null: Fiber | null), phase: (null: LifeCyclePhase | null), - + resetCurrentFiber, + setCurrentFiber, getCurrentFiberOwnerName, getCurrentFiberStackAddendum, }; diff --git a/src/renderers/shared/fiber/ReactFiberBeginWork.js b/src/renderers/shared/fiber/ReactFiberBeginWork.js index 32f91e7d178c0..a2883cf7ddb46 100644 --- a/src/renderers/shared/fiber/ReactFiberBeginWork.js +++ b/src/renderers/shared/fiber/ReactFiberBeginWork.js @@ -217,9 +217,9 @@ module.exports = function( if (__DEV__) { ReactCurrentOwner.current = workInProgress; - ReactDebugCurrentFiber.phase = 'render'; + ReactDebugCurrentFiber.setCurrentFiber(workInProgress, 'render'); nextChildren = fn(nextProps, context); - ReactDebugCurrentFiber.phase = null; + ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null); } else { nextChildren = fn(nextProps, context); } @@ -286,9 +286,9 @@ module.exports = function( ReactCurrentOwner.current = workInProgress; let nextChildren; if (__DEV__) { - ReactDebugCurrentFiber.phase = 'render'; + ReactDebugCurrentFiber.setCurrentFiber(workInProgress, 'render'); nextChildren = instance.render(); - ReactDebugCurrentFiber.phase = null; + ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null); } else { nextChildren = instance.render(); } @@ -725,7 +725,7 @@ module.exports = function( } if (__DEV__) { - ReactDebugCurrentFiber.current = workInProgress; + ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null); } switch (workInProgress.tag) { diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js index 74ddba49a777e..c733f036933da 100644 --- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js +++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js @@ -187,7 +187,7 @@ module.exports = function( renderPriority: PriorityLevel, ): Fiber | null { if (__DEV__) { - ReactDebugCurrentFiber.current = workInProgress; + ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null); } // Get the latest props. diff --git a/src/renderers/shared/fiber/ReactFiberContext.js b/src/renderers/shared/fiber/ReactFiberContext.js index 8a2c606409ceb..6ec37396f0841 100644 --- a/src/renderers/shared/fiber/ReactFiberContext.js +++ b/src/renderers/shared/fiber/ReactFiberContext.js @@ -26,7 +26,6 @@ const {createCursor, pop, push} = require('ReactFiberStack'); if (__DEV__) { var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber'); - var {ReactDebugCurrentFrame} = require('ReactGlobalSharedState'); var {startPhaseTimer, stopPhaseTimer} = require('ReactDebugFiberPerf'); var warnedAboutMissingGetChildContext = {}; } @@ -92,15 +91,15 @@ exports.getMaskedContext = function( if (__DEV__) { const name = getComponentName(workInProgress) || 'Unknown'; - ReactDebugCurrentFrame.current = workInProgress; + ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null); checkPropTypes( contextTypes, context, 'context', name, - ReactDebugCurrentFrame.getStackAddendum, + ReactDebugCurrentFiber.getCurrentFiberStackAddendum, ); - ReactDebugCurrentFrame.current = null; + ReactDebugCurrentFiber.resetCurrentFiber(); } // Cache unmasked context so we can avoid recreating masked context unless necessary. @@ -181,11 +180,11 @@ function processChildContext( let childContext; if (__DEV__) { - ReactDebugCurrentFiber.phase = 'getChildContext'; + ReactDebugCurrentFiber.setCurrentFiber(fiber, 'getChildContext'); startPhaseTimer(fiber, 'getChildContext'); childContext = instance.getChildContext(); stopPhaseTimer(); - ReactDebugCurrentFiber.phase = null; + ReactDebugCurrentFiber.resetCurrentFiber(); } else { childContext = instance.getChildContext(); } @@ -205,15 +204,15 @@ function processChildContext( // assume anything about the given fiber. We won't pass it down if we aren't sure. // TODO: remove this hack when we delete unstable_renderSubtree in Fiber. const workInProgress = isReconciling ? fiber : null; - ReactDebugCurrentFrame.current = workInProgress; + ReactDebugCurrentFiber.setCurrentFiber(workInProgress, null); checkPropTypes( childContextTypes, childContext, 'child context', name, - ReactDebugCurrentFrame.getStackAddendum, + ReactDebugCurrentFiber.getCurrentFiberStackAddendum, ); - ReactDebugCurrentFrame.current = null; + ReactDebugCurrentFiber.resetCurrentFiber(); } return {...parentContext, ...childContext}; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 998613be999e2..59799225b6b90 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -322,7 +322,7 @@ module.exports = function( function commitAllHostEffects() { while (nextEffect !== null) { if (__DEV__) { - ReactDebugCurrentFiber.current = nextEffect; + ReactDebugCurrentFiber.setCurrentFiber(nextEffect, null); recordEffect(); } @@ -383,7 +383,7 @@ module.exports = function( } if (__DEV__) { - ReactDebugCurrentFiber.current = null; + ReactDebugCurrentFiber.resetCurrentFiber(); } } @@ -711,7 +711,7 @@ module.exports = function( ReactCurrentOwner.current = null; if (__DEV__) { - ReactDebugCurrentFiber.current = null; + ReactDebugCurrentFiber.resetCurrentFiber(); } return next; @@ -740,7 +740,7 @@ module.exports = function( ReactCurrentOwner.current = null; if (__DEV__) { - ReactDebugCurrentFiber.current = null; + ReactDebugCurrentFiber.resetCurrentFiber(); } return next; @@ -1024,8 +1024,7 @@ module.exports = function( // It is no longer valid because we exited the user code. ReactCurrentOwner.current = null; if (__DEV__) { - ReactDebugCurrentFiber.current = null; - ReactDebugCurrentFiber.phase = null; + ReactDebugCurrentFiber.resetCurrentFiber(); } // It is no longer valid because this unit of work failed. nextUnitOfWork = null; From 7ee042442071d44b1e43dfad66fb218db0dab904 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 4 Jul 2017 01:54:07 -0700 Subject: [PATCH 3/5] Make ReactDebugCurrentFrame implementation independent Introduced ReactDebugCurrentStack for the Stack renderer which does the same thing as ReactDebugCurrentFiber. ReactDebugCurrentFrame no longer keeps track of the current fiber/debug id. That's handled by the individual renderers. Instead, it is now used to keep track of the current *implementation* of the current stack frame. That way it is decoupled from the specifics of the renderers. There can be multiple renderers in a context. What matters is which one is currently executing a debuggable context (such as a render function). --- .../classic/element/ReactDebugCurrentFrame.js | 34 +++-------------- .../shared/hooks/ReactDOMInvalidARIAHook.js | 11 ++++-- .../hooks/ReactDOMNullInputValuePropHook.js | 11 ++++-- .../hooks/ReactDOMUnknownPropertyHook.js | 14 +++++-- .../shared/fiber/ReactDebugCurrentFiber.js | 4 +- .../reconciler/ReactCompositeComponent.js | 17 +++++++-- .../reconciler/ReactDebugCurrentStack.js | 38 +++++++++++++++++++ .../stack/reconciler/ReactMultiChild.js | 10 +++++ 8 files changed, 93 insertions(+), 46 deletions(-) create mode 100644 src/renderers/shared/stack/reconciler/ReactDebugCurrentStack.js diff --git a/src/isomorphic/classic/element/ReactDebugCurrentFrame.js b/src/isomorphic/classic/element/ReactDebugCurrentFrame.js index 99d539abef1d9..fad26ea155693 100644 --- a/src/isomorphic/classic/element/ReactDebugCurrentFrame.js +++ b/src/isomorphic/classic/element/ReactDebugCurrentFrame.js @@ -12,42 +12,18 @@ 'use strict'; -import type {Fiber} from 'ReactFiber'; -import type {DebugID} from 'ReactInstanceType'; - const ReactDebugCurrentFrame = {}; if (__DEV__) { - var { - getStackAddendumByID, - getCurrentStackAddendum, - } = require('ReactComponentTreeHook'); - var { - getStackAddendumByWorkInProgressFiber, - } = require('ReactFiberComponentTreeHook'); - // Component that is being worked on - ReactDebugCurrentFrame.current = (null: Fiber | DebugID | null); + ReactDebugCurrentFrame.getCurrentStack = (null: null | (() => string | null)); ReactDebugCurrentFrame.getStackAddendum = function(): string | null { - let stack = null; - const current = ReactDebugCurrentFrame.current; - if (current !== null) { - if (typeof current === 'number') { - // DebugID from Stack. - const debugID = current; - stack = getStackAddendumByID(debugID); - } else if (typeof current.tag === 'number') { - // This is a Fiber. - // The stack will only be correct if this is a work in progress - // version and we're calling it during reconciliation. - const workInProgress = current; - stack = getStackAddendumByWorkInProgressFiber(workInProgress); - } - } else { - stack = getCurrentStackAddendum(); + const impl = ReactDebugCurrentFrame.getCurrentStack; + if (impl) { + return impl(); } - return stack; + return null; }; } diff --git a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js index 7eaf0bc1ac3b5..686089e21cd06 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMInvalidARIAHook.js @@ -12,7 +12,6 @@ 'use strict'; var DOMProperty = require('DOMProperty'); -var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber'); var warning = require('fbjs/lib/warning'); @@ -20,7 +19,10 @@ var warnedProperties = {}; var rARIA = new RegExp('^(aria)-[' + DOMProperty.ATTRIBUTE_NAME_CHAR + ']*$'); if (__DEV__) { - var {ReactComponentTreeHook} = require('ReactGlobalSharedState'); + var { + ReactComponentTreeHook, + ReactDebugCurrentFrame, + } = require('ReactGlobalSharedState'); var {getStackAddendumByID} = ReactComponentTreeHook; } @@ -29,8 +31,9 @@ function getStackAddendum(debugID) { // This can only happen on Stack return getStackAddendumByID(debugID); } else { - // This can only happen on Fiber - return ReactDebugCurrentFiber.getCurrentFiberStackAddendum(); + // This can only happen on Fiber / Server + var stack = ReactDebugCurrentFrame.getStackAddendum(); + return stack != null ? stack : ''; } } diff --git a/src/renderers/dom/shared/hooks/ReactDOMNullInputValuePropHook.js b/src/renderers/dom/shared/hooks/ReactDOMNullInputValuePropHook.js index 6b722b48641af..d0e27d212e14c 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMNullInputValuePropHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMNullInputValuePropHook.js @@ -11,11 +11,13 @@ 'use strict'; -var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber'); var warning = require('fbjs/lib/warning'); if (__DEV__) { - var {ReactComponentTreeHook} = require('ReactGlobalSharedState'); + var { + ReactComponentTreeHook, + ReactDebugCurrentFrame, + } = require('ReactGlobalSharedState'); var {getStackAddendumByID} = ReactComponentTreeHook; } @@ -26,8 +28,9 @@ function getStackAddendum(debugID) { // This can only happen on Stack return getStackAddendumByID(debugID); } else { - // This can only happen on Fiber - return ReactDebugCurrentFiber.getCurrentFiberStackAddendum(); + // This can only happen on Fiber / Server + var stack = ReactDebugCurrentFrame.getStackAddendum(); + return stack != null ? stack : ''; } } diff --git a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js index 083ae3019e465..58096bfea22dd 100644 --- a/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js +++ b/src/renderers/dom/shared/hooks/ReactDOMUnknownPropertyHook.js @@ -13,18 +13,24 @@ var DOMProperty = require('DOMProperty'); var EventPluginRegistry = require('EventPluginRegistry'); -var ReactDebugCurrentFiber = require('ReactDebugCurrentFiber'); -var {ReactComponentTreeHook} = require('ReactGlobalSharedState'); var warning = require('fbjs/lib/warning'); +if (__DEV__) { + var { + ReactComponentTreeHook, + ReactDebugCurrentFrame, + } = require('ReactGlobalSharedState'); + var {getStackAddendumByID} = ReactComponentTreeHook; +} + function getStackAddendum(debugID) { if (debugID != null) { // This can only happen on Stack - return ReactComponentTreeHook.getStackAddendumByID(debugID); + return getStackAddendumByID(debugID); } else { // This can only happen on Fiber / Server - var stack = ReactDebugCurrentFiber.getCurrentFiberStackAddendum(); + var stack = ReactDebugCurrentFrame.getStackAddendum(); return stack != null ? stack : ''; } } diff --git a/src/renderers/shared/fiber/ReactDebugCurrentFiber.js b/src/renderers/shared/fiber/ReactDebugCurrentFiber.js index 4685a8c0d6d40..dd36659201f24 100644 --- a/src/renderers/shared/fiber/ReactDebugCurrentFiber.js +++ b/src/renderers/shared/fiber/ReactDebugCurrentFiber.js @@ -52,13 +52,13 @@ function getCurrentFiberStackAddendum(): string | null { } function resetCurrentFiber() { - ReactDebugCurrentFrame.current = null; + ReactDebugCurrentFrame.getCurrentStack = null; ReactDebugCurrentFiber.current = null; ReactDebugCurrentFiber.phase = null; } function setCurrentFiber(fiber: Fiber | null, phase: LifeCyclePhase | null) { - ReactDebugCurrentFrame.current = fiber; + ReactDebugCurrentFrame.getCurrentStack = getCurrentFiberStackAddendum; ReactDebugCurrentFiber.current = fiber; ReactDebugCurrentFiber.phase = phase; } diff --git a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js index 434f67a4fbef3..527219740b900 100644 --- a/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js +++ b/src/renderers/shared/stack/reconciler/ReactCompositeComponent.js @@ -23,6 +23,7 @@ var {ReactCurrentOwner} = require('ReactGlobalSharedState'); if (__DEV__) { var {ReactDebugCurrentFrame} = require('ReactGlobalSharedState'); + var ReactDebugCurrentStack = require('ReactDebugCurrentStack'); var warningAboutMissingGetChildContext = {}; } @@ -403,6 +404,8 @@ var ReactCompositeComponent = { ) { if (__DEV__) { ReactCurrentOwner.current = this; + ReactDebugCurrentFrame.getCurrentStack = + ReactDebugCurrentStack.getStackAddendum; try { return this._constructComponentWithoutOwner( doConstruct, @@ -412,6 +415,7 @@ var ReactCompositeComponent = { ); } finally { ReactCurrentOwner.current = null; + ReactDebugCurrentFrame.getCurrentStack = null; } } else { return this._constructComponentWithoutOwner( @@ -746,15 +750,15 @@ var ReactCompositeComponent = { */ _checkContextTypes: function(typeSpecs, values, location: string) { if (__DEV__) { - ReactDebugCurrentFrame.current = this._debugID; + ReactDebugCurrentStack.current = this._debugID; checkPropTypes( typeSpecs, values, location, this.getName(), - ReactDebugCurrentFrame.getStackAddendum, + ReactDebugCurrentStack.getStackAddendum, ); - ReactDebugCurrentFrame.current = null; + ReactDebugCurrentStack.current = null; } }, @@ -1246,10 +1250,17 @@ var ReactCompositeComponent = { this._compositeType !== ReactCompositeComponentTypes.StatelessFunctional ) { ReactCurrentOwner.current = this; + if (__DEV__) { + ReactDebugCurrentFrame.getCurrentStack = + ReactDebugCurrentStack.getStackAddendum; + } try { renderedElement = this._renderValidatedComponentWithoutOwnerOrContext(); } finally { ReactCurrentOwner.current = null; + if (__DEV__) { + ReactDebugCurrentFrame.getCurrentStack = null; + } } } else { renderedElement = this._renderValidatedComponentWithoutOwnerOrContext(); diff --git a/src/renderers/shared/stack/reconciler/ReactDebugCurrentStack.js b/src/renderers/shared/stack/reconciler/ReactDebugCurrentStack.js new file mode 100644 index 0000000000000..c7d4c7a312ada --- /dev/null +++ b/src/renderers/shared/stack/reconciler/ReactDebugCurrentStack.js @@ -0,0 +1,38 @@ +/** + * 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 ReactDebugCurrentStack + * @flow + */ + +'use strict'; + +import type {DebugID} from 'ReactInstanceType'; + +const ReactDebugCurrentStack = {}; + +if (__DEV__) { + var {ReactComponentTreeHook} = require('ReactGlobalSharedState'); + var {getStackAddendumByID, getCurrentStackAddendum} = ReactComponentTreeHook; + + // Component that is being worked on + ReactDebugCurrentStack.current = (null: DebugID | null); + + ReactDebugCurrentStack.getStackAddendum = function(): string | null { + let stack = null; + const current = ReactDebugCurrentStack.current; + if (current !== null) { + stack = getStackAddendumByID(current); + } else { + stack = getCurrentStackAddendum(); + } + return stack; + }; +} + +module.exports = ReactDebugCurrentStack; diff --git a/src/renderers/shared/stack/reconciler/ReactMultiChild.js b/src/renderers/shared/stack/reconciler/ReactMultiChild.js index 8bde4e26ffffa..be03aeb44c57f 100644 --- a/src/renderers/shared/stack/reconciler/ReactMultiChild.js +++ b/src/renderers/shared/stack/reconciler/ReactMultiChild.js @@ -18,6 +18,10 @@ var ReactInstrumentation = require('ReactInstrumentation'); var ReactReconciler = require('ReactReconciler'); var ReactChildReconciler = require('ReactChildReconciler'); var {ReactCurrentOwner} = require('ReactGlobalSharedState'); +if (__DEV__) { + var {ReactDebugCurrentFrame} = require('ReactGlobalSharedState'); + var ReactDebugCurrentStack = require('ReactDebugCurrentStack'); +} var emptyFunction = require('fbjs/lib/emptyFunction'); var flattenStackChildren = require('flattenStackChildren'); @@ -179,6 +183,8 @@ var ReactMultiChild = { if (this._currentElement) { try { ReactCurrentOwner.current = this._currentElement._owner; + ReactDebugCurrentFrame.getCurrentStack = + ReactDebugCurrentStack.getStackAddendum; return ReactChildReconciler.instantiateChildren( nestedChildren, transaction, @@ -187,6 +193,7 @@ var ReactMultiChild = { ); } finally { ReactCurrentOwner.current = null; + ReactDebugCurrentFrame.getCurrentStack = null; } } } @@ -212,12 +219,15 @@ var ReactMultiChild = { if (this._currentElement) { try { ReactCurrentOwner.current = this._currentElement._owner; + ReactDebugCurrentFrame.getCurrentStack = + ReactDebugCurrentStack.getStackAddendum; nextChildren = flattenStackChildren( nextNestedChildrenElements, selfDebugID, ); } finally { ReactCurrentOwner.current = null; + ReactDebugCurrentFrame.getCurrentStack = null; } ReactChildReconciler.updateChildren( prevChildren, From f850b59cc6bcd644e5cdfc4c21eecceb2c078794 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 4 Jul 2017 03:17:01 -0700 Subject: [PATCH 4/5] Add debug frames to ReactPartialRenderer (ssr) Basic functionality. --- scripts/fiber/tests-passing-except-dev.txt | 3 - scripts/fiber/tests-passing.txt | 3 + .../shared/server/ReactPartialRenderer.js | 57 ++++++++++++++++--- 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt index e95c7f7c7741e..457b16d592062 100644 --- a/scripts/fiber/tests-passing-except-dev.txt +++ b/scripts/fiber/tests-passing-except-dev.txt @@ -1,5 +1,2 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js -* gives source code refs for unknown prop warning (ssr) -* gives source code refs for unknown prop warning for exact elements (ssr) * gives source code refs for unknown prop warning for exact elements in composition (ssr) -* should suggest property name if available (ssr) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 7006c289b0658..10ca5eaaffd6c 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -853,10 +853,13 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should warn about props that are no longer supported * should warn about props that are no longer supported (ssr) * gives source code refs for unknown prop warning +* gives source code refs for unknown prop warning (ssr) * gives source code refs for unknown prop warning for update render * gives source code refs for unknown prop warning for exact elements +* gives source code refs for unknown prop warning for exact elements (ssr) * gives source code refs for unknown prop warning for exact elements in composition * should suggest property name if available +* should suggest property name if available (ssr) * renders innerHTML and preserves whitespace * render and then updates innerHTML and preserves whitespace diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index 0aded2bbf07f0..7e4c36ab36cdc 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -42,6 +42,49 @@ if (__DEV__) { validateInputPropertes(type, props); validateUnknownPropertes(type, props); }; + + var describeComponentFrame = require('describeComponentFrame'); + var describeStackFrame = function( + frame: { + tag?: string, + children: Array<*>, + childIndex: number, + }, + ): string { + var element = frame.children[frame.childIndex - 1]; + if (!element) { + return ''; + } + var source = element._source; + var type = element.type; + var name = typeof type === 'string' + ? type + : typeof type === 'function' ? type.displayName || type.name : null; + var ownerName = null; + return describeComponentFrame(name, source, ownerName); + }; + + var {ReactDebugCurrentFrame} = require('ReactGlobalSharedState'); + var currentDebugStack = null; + var setCurrentDebugStack = function(stack) { + currentDebugStack = stack; + ReactDebugCurrentFrame.getCurrentStack = getStackAddendum; + }; + var resetCurrentDebugStack = function() { + currentDebugStack = null; + ReactDebugCurrentFrame.getCurrentStack = null; + }; + var getStackAddendum = function(): null | string { + if (currentDebugStack === null) { + return null; + } + let stack = ''; + let debugStack = currentDebugStack; + for (let i = debugStack.length - 1; i >= 0; i--) { + stack += describeStackFrame(debugStack[i]); + } + return stack; + }; } var didWarnDefaultInputValue = false; @@ -145,13 +188,7 @@ function maskContext(type, context) { function checkContextTypes(typeSpecs, values, location: string) { if (__DEV__) { - checkPropTypes( - typeSpecs, - values, - location, - 'Component', - () => '', // ReactDebugCurrentFrame.getStackAddendum, - ); + checkPropTypes(typeSpecs, values, location, 'Component', getStackAddendum); } } @@ -374,7 +411,13 @@ class ReactDOMServerRenderer { continue; } var child = frame.children[frame.childIndex++]; + if (__DEV__) { + setCurrentDebugStack(this.stack); + } out += this.render(child, frame.context); + if (__DEV__) { + resetCurrentDebugStack(); + } } return out; } From 8ca272d684a9d183ae648e9d5b20e9f53c5adc3b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 14 Jul 2017 15:23:09 -0700 Subject: [PATCH 5/5] Add shared modules to shallow renderer This is now needed because we share describeComponentFrame. --- scripts/rollup/bundles.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/rollup/bundles.js b/scripts/rollup/bundles.js index 580a5571f4bb0..48e314a551aef 100644 --- a/scripts/rollup/bundles.js +++ b/scripts/rollup/bundles.js @@ -381,7 +381,11 @@ const bundles = [ label: 'shallow-renderer', manglePropertiesOnProd: false, name: 'react-test-renderer/shallow', - paths: ['src/renderers/shared/**/*.js', 'src/renderers/testing/**/*.js'], + paths: [ + 'src/renderers/shared/**/*.js', + 'src/renderers/testing/**/*.js', + 'src/shared/**/*.js', + ], }, /******* React Noop Renderer (used only for fixtures/fiber-debugger) *******/