diff --git a/.eslintrc.js b/.eslintrc.js index 0197c75b7688..cf5b58587085 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -486,6 +486,7 @@ module.exports = { $ReadOnlyArray: 'readonly', $ArrayBufferView: 'readonly', $Shape: 'readonly', + ConsoleTask: 'readonly', // TOOD: Figure out what the official name of this will be. ReturnType: 'readonly', AnimationFrameID: 'readonly', // For Flow type annotation. Only `BigInt` is valid at runtime. diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index b7a73eb2ab57..bc660a207f07 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -1123,10 +1123,11 @@ describe('ReactFlight', () => { } function App() { - return ( - - - + // We use the ReactServer runtime here to get the Server owner. + return ReactServer.createElement( + Indirection, + null, + ReactServer.createElement(ClientComponent), ); } @@ -1143,11 +1144,10 @@ describe('ReactFlight', () => { '\n' + 'Check the render method of `Component`. See https://react.dev/link/warning-keys for more information.\n' + ' in span (at **)\n' + - // TODO: Because this validates after the div has been mounted, it is part of - // the parent stack but since owner stacks will switch to owners this goes away again. - (gate(flags => flags.enableOwnerStacks) ? ' in div (at **)\n' : '') + ' in Component (at **)\n' + - ' in Indirection (at **)\n' + + (gate(flags => flags.enableOwnerStacks) + ? '' + : ' in Indirection (at **)\n') + ' in App (at **)', ); }); diff --git a/packages/react-dom-bindings/src/client/validateDOMNesting.js b/packages/react-dom-bindings/src/client/validateDOMNesting.js index ab49cb4f4b02..b7e99757cac5 100644 --- a/packages/react-dom-bindings/src/client/validateDOMNesting.js +++ b/packages/react-dom-bindings/src/client/validateDOMNesting.js @@ -7,6 +7,8 @@ * @flow */ +import {getCurrentParentStackInDev} from 'react-reconciler/src/ReactCurrentFiber'; + type Info = {tag: string}; export type AncestorInfoDev = { current: ?Info, @@ -476,19 +478,31 @@ function validateDOMNesting( ' Add a , or to your code to match the DOM tree generated by ' + 'the browser.'; } - console.error( - 'In HTML, %s cannot be a child of <%s>.%s\n' + - 'This will cause a hydration error.', + // Don't transform into consoleWithStackDev here because we add a manual stack. + // We use the parent stack here instead of the owner stack because the parent + // stack has more useful context for nesting. + // TODO: Format this as a linkified "diff view" with props instead of + // a stack trace since the stack trace format is now for owner stacks. + console['error']( + 'Warning: In HTML, %s cannot be a child of <%s>.%s\n' + + 'This will cause a hydration error.%s', tagDisplayName, ancestorTag, info, + getCurrentParentStackInDev(), ); } else { - console.error( - 'In HTML, %s cannot be a descendant of <%s>.\n' + - 'This will cause a hydration error.', + // Don't transform into consoleWithStackDev here because we add a manual stack. + // We use the parent stack here instead of the owner stack because the parent + // stack has more useful context for nesting. + // TODO: Format this as a linkified "diff view" with props instead of + // a stack trace since the stack trace format is now for owner stacks. + console['error']( + 'Warning: In HTML, %s cannot be a descendant of <%s>.\n' + + 'This will cause a hydration error.%s', tagDisplayName, ancestorTag, + getCurrentParentStackInDev(), ); } return false; @@ -510,18 +524,30 @@ function validateTextNesting(childText: string, parentTag: string): boolean { didWarn[warnKey] = true; if (/\S/.test(childText)) { - console.error( - 'In HTML, text nodes cannot be a child of <%s>.\n' + - 'This will cause a hydration error.', + // Don't transform into consoleWithStackDev here because we add a manual stack. + // We use the parent stack here instead of the owner stack because the parent + // stack has more useful context for nesting. + // TODO: Format this as a linkified "diff view" with props instead of + // a stack trace since the stack trace format is now for owner stacks. + console['error']( + 'Warning: In HTML, text nodes cannot be a child of <%s>.\n' + + 'This will cause a hydration error.%s', parentTag, + getCurrentParentStackInDev(), ); } else { - console.error( - 'In HTML, whitespace text nodes cannot be a child of <%s>. ' + + // Don't transform into consoleWithStackDev here because we add a manual stack. + // We use the parent stack here instead of the owner stack because the parent + // stack has more useful context for nesting. + // TODO: Format this as a linkified "diff view" with props instead of + // a stack trace since the stack trace format is now for owner stacks. + console['error']( + 'Warning: In HTML, whitespace text nodes cannot be a child of <%s>. ' + "Make sure you don't have any extra whitespace between tags on " + 'each line of your source code.\n' + - 'This will cause a hydration error.', + 'This will cause a hydration error.%s', parentTag, + getCurrentParentStackInDev(), ); } return false; diff --git a/packages/react-dom/src/__tests__/ReactChildReconciler-test.js b/packages/react-dom/src/__tests__/ReactChildReconciler-test.js index ef584a785692..425f4abd1334 100644 --- a/packages/react-dom/src/__tests__/ReactChildReconciler-test.js +++ b/packages/react-dom/src/__tests__/ReactChildReconciler-test.js @@ -130,7 +130,9 @@ describe('ReactChildReconciler', () => { 'could change in a future version.\n' + ' in div (at **)\n' + ' in Component (at **)\n' + - ' in Parent (at **)\n' + + (gate(flags => flags.enableOwnerStacks) + ? '' + : ' in Parent (at **)\n') + ' in GrandParent (at **)', ); }); @@ -189,7 +191,9 @@ describe('ReactChildReconciler', () => { 'could change in a future version.\n' + ' in div (at **)\n' + ' in Component (at **)\n' + - ' in Parent (at **)\n' + + (gate(flags => flags.enableOwnerStacks) + ? '' + : ' in Parent (at **)\n') + ' in GrandParent (at **)', ); }); diff --git a/packages/react-dom/src/__tests__/ReactComponent-test.js b/packages/react-dom/src/__tests__/ReactComponent-test.js index 61538fed6949..1951f4ba7340 100644 --- a/packages/react-dom/src/__tests__/ReactComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactComponent-test.js @@ -761,7 +761,9 @@ describe('ReactComponent', () => { 'Or maybe you meant to call this function rather than return it.\n' + ' {Foo}\n' + ' in span (at **)\n' + - ' in div (at **)\n' + + (gate(flags => flags.enableOwnerStacks) + ? '' + : ' in div (at **)\n') + ' in Foo (at **)', ); }); @@ -820,7 +822,9 @@ describe('ReactComponent', () => { 'Or maybe you meant to call this function rather than return it.\n' + ' {Foo}\n' + ' in span (at **)\n' + - ' in div (at **)\n' + + (gate(flags => flags.enableOwnerStacks) + ? '' + : ' in div (at **)\n') + ' in Foo (at **)', ]); await act(() => { diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index 72fa57fb1fd5..697b65dd6075 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -552,7 +552,7 @@ describe('ReactDOM', () => { // ReactDOM(App > div > span) 'Invalid ARIA attribute `ariaTypo`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' + ' in span (at **)\n' + - ' in div (at **)\n' + + (gate(flags => flags.enableOwnerStacks) ? '' : ' in div (at **)\n') + ' in App (at **)', // ReactDOM(App > div > ServerEntry) >>> ReactDOMServer(Child) >>> ReactDOMServer(App2) >>> ReactDOMServer(blink) 'Invalid ARIA attribute `ariaTypo2`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' + @@ -569,7 +569,7 @@ describe('ReactDOM', () => { // ReactDOM(App > div > font) 'Invalid ARIA attribute `ariaTypo5`. ARIA attributes follow the pattern aria-* and must be lowercase.\n' + ' in font (at **)\n' + - ' in div (at **)\n' + + (gate(flags => flags.enableOwnerStacks) ? '' : ' in div (at **)\n') + ' in App (at **)', ]); }); diff --git a/packages/react-dom/src/__tests__/ReactMultiChild-test.js b/packages/react-dom/src/__tests__/ReactMultiChild-test.js index 44d71fe7fc80..1ffc793f9602 100644 --- a/packages/react-dom/src/__tests__/ReactMultiChild-test.js +++ b/packages/react-dom/src/__tests__/ReactMultiChild-test.js @@ -229,7 +229,9 @@ describe('ReactMultiChild', () => { 'could change in a future version.\n' + ' in div (at **)\n' + ' in WrapperComponent (at **)\n' + - ' in div (at **)\n' + + (gate(flags => flags.enableOwnerStacks) + ? '' + : ' in div (at **)\n') + ' in Parent (at **)', ); }); @@ -292,7 +294,9 @@ describe('ReactMultiChild', () => { 'could change in a future version.\n' + ' in div (at **)\n' + ' in WrapperComponent (at **)\n' + - ' in div (at **)\n' + + (gate(flags => flags.enableOwnerStacks) + ? '' + : ' in div (at **)\n') + ' in Parent (at **)', ); }); diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js index 9a10a8d061b5..1c40e72a7853 100644 --- a/packages/react-dom/src/__tests__/ReactUpdates-test.js +++ b/packages/react-dom/src/__tests__/ReactUpdates-test.js @@ -1848,7 +1848,7 @@ describe('ReactUpdates', () => { it('warns about a deferred infinite update loop with useEffect', async () => { function NonTerminating() { const [step, setStep] = React.useState(0); - React.useEffect(() => { + React.useEffect(function myEffect() { setStep(x => x + 1); }); return step; @@ -1860,10 +1860,12 @@ describe('ReactUpdates', () => { let error = null; let stack = null; + let nativeStack = null; const originalConsoleError = console.error; console.error = (e, s) => { error = e; stack = s; + nativeStack = new Error().stack; Scheduler.log('stop'); }; try { @@ -1876,7 +1878,14 @@ describe('ReactUpdates', () => { } expect(error).toContain('Maximum update depth exceeded'); - expect(stack).toContain('at NonTerminating'); + // The currently executing effect should be on the native stack + expect(nativeStack).toContain('at myEffect'); + if (!gate(flags => flags.enableOwnerStacks)) { + // The currently running component's name is not in the owner + // stack because it's just its JSX callsite. + expect(stack).toContain('at NonTerminating'); + } + expect(stack).toContain('at App'); }); it('can have nested updates if they do not cross the limit', async () => { diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 0f14af18af18..3346c739536d 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -61,6 +61,7 @@ import {getIsHydrating} from './ReactFiberHydrationContext'; import {pushTreeFork} from './ReactFiberTreeContext'; import {createThenableState, trackUsedThenable} from './ReactFiberThenable'; import {readContextDuringReconciliation} from './ReactFiberNewContext'; +import {callLazyInitInDEV} from './ReactFiberCallUserSpace'; import { getCurrentFiber as getCurrentDebugFiberInDEV, @@ -362,6 +363,9 @@ function warnOnSymbolType(returnFiber: Fiber, invalidChild: symbol) { } function resolveLazy(lazyType: any) { + if (__DEV__) { + return callLazyInitInDEV(lazyType); + } const payload = lazyType._payload; const init = lazyType._init; return init(payload); @@ -683,11 +687,17 @@ function createChildReconciler( return created; } case REACT_LAZY_TYPE: { - const payload = newChild._payload; - const init = newChild._init; + let resolvedChild; + if (__DEV__) { + resolvedChild = callLazyInitInDEV(newChild); + } else { + const payload = newChild._payload; + const init = newChild._init; + resolvedChild = init(payload); + } return createChild( returnFiber, - init(payload), + resolvedChild, lanes, mergeDebugInfo(debugInfo, newChild._debugInfo), // call merge after init ); @@ -811,12 +821,18 @@ function createChildReconciler( } } case REACT_LAZY_TYPE: { - const payload = newChild._payload; - const init = newChild._init; + let resolvedChild; + if (__DEV__) { + resolvedChild = callLazyInitInDEV(newChild); + } else { + const payload = newChild._payload; + const init = newChild._init; + resolvedChild = init(payload); + } return updateSlot( returnFiber, oldFiber, - init(payload), + resolvedChild, lanes, mergeDebugInfo(debugInfo, newChild._debugInfo), ); @@ -937,17 +953,24 @@ function createChildReconciler( debugInfo, ); } - case REACT_LAZY_TYPE: - const payload = newChild._payload; - const init = newChild._init; + case REACT_LAZY_TYPE: { + let resolvedChild; + if (__DEV__) { + resolvedChild = callLazyInitInDEV(newChild); + } else { + const payload = newChild._payload; + const init = newChild._init; + resolvedChild = init(payload); + } return updateFromMap( existingChildren, returnFiber, newIdx, - init(payload), + resolvedChild, lanes, mergeDebugInfo(debugInfo, newChild._debugInfo), ); + } } if ( @@ -1047,11 +1070,18 @@ function createChildReconciler( key, ); break; - case REACT_LAZY_TYPE: - const payload = child._payload; - const init = (child._init: any); - warnOnInvalidKey(init(payload), knownKeys, returnFiber); + case REACT_LAZY_TYPE: { + let resolvedChild; + if (__DEV__) { + resolvedChild = callLazyInitInDEV((child: any)); + } else { + const payload = child._payload; + const init = (child._init: any); + resolvedChild = init(payload); + } + warnOnInvalidKey(resolvedChild, knownKeys, returnFiber); break; + } default: break; } diff --git a/packages/react-reconciler/src/ReactCurrentFiber.js b/packages/react-reconciler/src/ReactCurrentFiber.js index c0ff5d16df4c..fa2a11c00a8d 100644 --- a/packages/react-reconciler/src/ReactCurrentFiber.js +++ b/packages/react-reconciler/src/ReactCurrentFiber.js @@ -10,8 +10,12 @@ import type {Fiber} from './ReactInternalTypes'; import ReactSharedInternals from 'shared/ReactSharedInternals'; -import {getStackByFiberInDevAndProd} from './ReactFiberComponentStack'; +import { + getStackByFiberInDevAndProd, + getOwnerStackByFiberInDev, +} from './ReactFiberComponentStack'; import {getComponentNameFromOwner} from 'react-reconciler/src/getComponentNameFromFiber'; +import {enableOwnerStacks} from 'shared/ReactFeatureFlags'; export let current: Fiber | null = null; export let isRendering: boolean = false; @@ -29,6 +33,17 @@ export function getCurrentFiberOwnerNameInDevOrNull(): string | null { return null; } +export function getCurrentParentStackInDev(): string { + // This is used to get the parent stack even with owner stacks turned on. + if (__DEV__) { + if (current === null) { + return ''; + } + return getStackByFiberInDevAndProd(current); + } + return ''; +} + function getCurrentFiberStackInDev(): string { if (__DEV__) { if (current === null) { @@ -36,6 +51,11 @@ function getCurrentFiberStackInDev(): string { } // Safe because if current fiber exists, we are reconciling, // and it is guaranteed to be the work-in-progress version. + // TODO: The above comment is not actually true. We might be + // in a commit phase or preemptive set state callback. + if (enableOwnerStacks) { + return getOwnerStackByFiberInDev(current); + } return getStackByFiberInDevAndProd(current); } return ''; diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 03bdd13f56eb..7234117a50c7 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -38,6 +38,7 @@ import { enableDO_NOT_USE_disableStrictPassiveEffect, enableRenderableContext, disableLegacyMode, + enableOwnerStacks, } from 'shared/ReactFeatureFlags'; import {NoFlags, Placement, StaticMask} from './ReactFiberFlags'; import {ConcurrentRoot} from './ReactRootTags'; @@ -205,6 +206,10 @@ function FiberNode( // This isn't directly used but is handy for debugging internals: this._debugInfo = null; this._debugOwner = null; + if (enableOwnerStacks) { + this._debugStack = null; + this._debugTask = null; + } this._debugNeedsRemount = false; this._debugHookTypes = null; if (!hasBadMapPolyfill && typeof Object.preventExtensions === 'function') { @@ -278,6 +283,10 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber { // DEV-only fields workInProgress._debugOwner = current._debugOwner; + if (enableOwnerStacks) { + workInProgress._debugStack = current._debugStack; + workInProgress._debugTask = current._debugTask; + } workInProgress._debugHookTypes = current._debugHookTypes; } @@ -683,6 +692,10 @@ export function createFiberFromElement( ); if (__DEV__) { fiber._debugOwner = element._owner; + if (enableOwnerStacks) { + fiber._debugStack = element._debugStack; + fiber._debugTask = element._debugTask; + } } return fiber; } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index e638580bb0da..6eeb7ab37797 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -110,6 +110,7 @@ import { disableLegacyMode, disableDefaultPropsExceptForClasses, disableStringRefs, + enableOwnerStacks, } from 'shared/ReactFeatureFlags'; import isArray from 'shared/isArray'; import shallowEqual from 'shared/shallowEqual'; @@ -124,7 +125,6 @@ import { } from 'shared/ReactSymbols'; import { getCurrentFiberOwnerNameInDevOrNull, - setIsRendering, setCurrentFiber, } from './ReactCurrentFiber'; import { @@ -297,6 +297,11 @@ import { pushRootMarkerInstance, TransitionTracingMarker, } from './ReactFiberTracingMarkerComponent'; +import { + callLazyInitInDEV, + callComponentInDEV, + callRenderInDEV, +} from './ReactFiberCallUserSpace'; // A special exception that's used to unwind the stack when an update flows // into a dehydrated boundary. @@ -432,7 +437,6 @@ function updateForwardRef( markComponentRenderStarted(workInProgress); } if (__DEV__) { - setIsRendering(true); nextChildren = renderWithHooks( current, workInProgress, @@ -442,7 +446,6 @@ function updateForwardRef( renderLanes, ); hasId = checkDidRenderIdHook(); - setIsRendering(false); } else { nextChildren = renderWithHooks( current, @@ -1149,7 +1152,6 @@ function updateFunctionComponent( markComponentRenderStarted(workInProgress); } if (__DEV__) { - setIsRendering(true); nextChildren = renderWithHooks( current, workInProgress, @@ -1159,7 +1161,6 @@ function updateFunctionComponent( renderLanes, ); hasId = checkDidRenderIdHook(); - setIsRendering(false); } else { nextChildren = renderWithHooks( current, @@ -1393,20 +1394,18 @@ function finishClassComponent( markComponentRenderStarted(workInProgress); } if (__DEV__) { - setIsRendering(true); - nextChildren = instance.render(); + nextChildren = callRenderInDEV(instance); if ( debugRenderPhaseSideEffectsForStrictMode && workInProgress.mode & StrictLegacyMode ) { setIsStrictModeForDevtools(true); try { - instance.render(); + callRenderInDEV(instance); } finally { setIsStrictModeForDevtools(false); } } - setIsRendering(false); } else { nextChildren = instance.render(); } @@ -1766,9 +1765,14 @@ function mountLazyComponent( const props = workInProgress.pendingProps; const lazyComponent: LazyComponentType = elementType; - const payload = lazyComponent._payload; - const init = lazyComponent._init; - let Component = init(payload); + let Component; + if (__DEV__) { + Component = callLazyInitInDEV(lazyComponent); + } else { + const payload = lazyComponent._payload; + const init = lazyComponent._init; + Component = init(payload); + } // Store the unwrapped component in the type. workInProgress.type = Component; @@ -3417,9 +3421,7 @@ function updateContextConsumer( } let newChildren; if (__DEV__) { - setIsRendering(true); - newChildren = render(newValue); - setIsRendering(false); + newChildren = callComponentInDEV(render, newValue, undefined); } else { newChildren = render(newValue); } @@ -3831,18 +3833,19 @@ function beginWork( if (__DEV__) { if (workInProgress._debugNeedsRemount && current !== null) { // This will restart the begin phase with a new fiber. - return remountFiber( - current, - workInProgress, - createFiberFromTypeAndProps( - workInProgress.type, - workInProgress.key, - workInProgress.pendingProps, - workInProgress._debugOwner || null, - workInProgress.mode, - workInProgress.lanes, - ), + const copiedFiber = createFiberFromTypeAndProps( + workInProgress.type, + workInProgress.key, + workInProgress.pendingProps, + workInProgress._debugOwner || null, + workInProgress.mode, + workInProgress.lanes, ); + if (enableOwnerStacks) { + copiedFiber._debugStack = workInProgress._debugStack; + copiedFiber._debugTask = workInProgress._debugTask; + } + return remountFiber(current, workInProgress, copiedFiber); } } diff --git a/packages/react-reconciler/src/ReactFiberCallUserSpace.js b/packages/react-reconciler/src/ReactFiberCallUserSpace.js new file mode 100644 index 000000000000..dfc88b64be8c --- /dev/null +++ b/packages/react-reconciler/src/ReactFiberCallUserSpace.js @@ -0,0 +1,46 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import type {LazyComponent} from 'react/src/ReactLazy'; + +import {setIsRendering} from './ReactCurrentFiber'; + +// These indirections exists so we can exclude its stack frame in DEV (and anything below it). +// TODO: Consider marking the whole bundle instead of these boundaries. + +/** @noinline */ +export function callComponentInDEV( + Component: (p: Props, arg: Arg) => R, + props: Props, + secondArg: Arg, +): R { + setIsRendering(true); + const result = Component(props, secondArg); + setIsRendering(false); + return result; +} + +interface ClassInstance { + render(): R; +} + +/** @noinline */ +export function callRenderInDEV(instance: ClassInstance): R { + setIsRendering(true); + const result = instance.render(); + setIsRendering(false); + return result; +} + +/** @noinline */ +export function callLazyInitInDEV(lazy: LazyComponent): any { + const payload = lazy._payload; + const init = lazy._init; + return init(payload); +} diff --git a/packages/react-reconciler/src/ReactFiberComponentStack.js b/packages/react-reconciler/src/ReactFiberComponentStack.js index 193c27aef1e1..e5e25f674690 100644 --- a/packages/react-reconciler/src/ReactFiberComponentStack.js +++ b/packages/react-reconciler/src/ReactFiberComponentStack.js @@ -7,7 +7,9 @@ * @flow */ +import {enableOwnerStacks} from 'shared/ReactFeatureFlags'; import type {Fiber} from './ReactInternalTypes'; +import type {ReactComponentInfo} from 'shared/ReactTypes'; import { HostComponent, @@ -20,6 +22,7 @@ import { ForwardRef, SimpleMemoComponent, ClassComponent, + HostText, } from './ReactWorkTags'; import { describeBuiltInComponentFrame, @@ -27,6 +30,7 @@ import { describeClassComponentFrame, describeDebugInfoFrame, } from 'shared/ReactComponentStackFrame'; +import {formatOwnerStack} from './ReactFiberOwnerStack'; function describeFiber(fiber: Fiber): string { switch (fiber.tag) { @@ -78,3 +82,104 @@ export function getStackByFiberInDevAndProd(workInProgress: Fiber): string { return '\nError generating stack: ' + x.message + '\n' + x.stack; } } + +function describeFunctionComponentFrameWithoutLineNumber(fn: Function): string { + // We use this because we don't actually want to describe the line of the component + // but just the component name. + const name = fn ? fn.displayName || fn.name : ''; + return name ? describeBuiltInComponentFrame(name) : ''; +} + +export function getOwnerStackByFiberInDev(workInProgress: Fiber): string { + if (!enableOwnerStacks || !__DEV__) { + return ''; + } + try { + let info = ''; + + if (workInProgress.tag === HostText) { + // Text nodes never have an owner/stack because they're not created through JSX. + // We use the parent since text nodes are always created through a host parent. + workInProgress = (workInProgress.return: any); + } + + // The owner stack of the current fiber will be where it was created, i.e. inside its owner. + // There's no actual name of the currently executing component. Instead, that is available + // on the regular stack that's currently executing. However, for built-ins there is no such + // named stack frame and it would be ignored as being internal anyway. Therefore we add + // add one extra frame just to describe the "current" built-in component by name. + // Similarly, if there is no owner at all, then there's no stack frame so we add the name + // of the root component to the stack to know which component is currently executing. + switch (workInProgress.tag) { + case HostHoistable: + case HostSingleton: + case HostComponent: + info += describeBuiltInComponentFrame(workInProgress.type); + break; + case SuspenseComponent: + info += describeBuiltInComponentFrame('Suspense'); + break; + case SuspenseListComponent: + info += describeBuiltInComponentFrame('SuspenseList'); + break; + case FunctionComponent: + case SimpleMemoComponent: + case ClassComponent: + if (!workInProgress._debugOwner) { + info += describeFunctionComponentFrameWithoutLineNumber( + workInProgress.type, + ); + } + break; + case ForwardRef: + if (!workInProgress._debugOwner) { + info += describeFunctionComponentFrameWithoutLineNumber( + workInProgress.type.render, + ); + } + break; + } + + let owner: void | null | Fiber | ReactComponentInfo = workInProgress; + + while (owner) { + if (typeof owner.tag === 'number') { + const fiber: Fiber = (owner: any); + owner = fiber._debugOwner; + let debugStack = fiber._debugStack; + // If we don't actually print the stack if there is no owner of this JSX element. + // In a real app it's typically not useful since the root app is always controlled + // by the framework. These also tend to have noisy stacks because they're not rooted + // in a React render but in some imperative bootstrapping code. It could be useful + // if the element was created in module scope. E.g. hoisted. We could add a a single + // stack frame for context for example but it doesn't say much if that's a wrapper. + if (owner && debugStack) { + if (typeof debugStack !== 'string') { + // Stash the formatted stack so that we can avoid redoing the filtering. + fiber._debugStack = debugStack = formatOwnerStack(debugStack); + } + if (debugStack !== '') { + info += '\n' + debugStack; + } + } + } else if (typeof owner.stack === 'string') { + // Server Component + // The Server Component stack can come from a different VM that formats it different. + // Likely V8. Since Chrome based browsers support createTask which is going to use + // another code path anyway. I.e. this is likely NOT a V8 based browser. + // This will cause some of the stack to have different formatting. + // TODO: Normalize server component stacks to the client formatting. + if (owner.stack !== '') { + info += '\n' + owner.stack; + } + const componentInfo: ReactComponentInfo = (owner: any); + owner = componentInfo.owner; + } else { + break; + } + } + return info; + } catch (x) { + return '\nError generating stack: ' + x.message + '\n' + x.stack; + } +} diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index d49b8367d099..f1c9c7a872b7 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -159,6 +159,8 @@ import { requestCurrentTransition, } from './ReactFiberTransition'; +import {callComponentInDEV} from './ReactFiberCallUserSpace'; + export type Update = { lane: Lane, revertLane: Lane, @@ -590,7 +592,9 @@ export function renderWithHooks( (workInProgress.mode & StrictLegacyMode) !== NoMode; shouldDoubleInvokeUserFnsInHooksDEV = shouldDoubleRenderDEV; - let children = Component(props, secondArg); + let children = __DEV__ + ? callComponentInDEV(Component, props, secondArg) + : Component(props, secondArg); shouldDoubleInvokeUserFnsInHooksDEV = false; // Check if there was a render phase update @@ -822,7 +826,9 @@ function renderWithHooksAgain( ? HooksDispatcherOnRerenderInDEV : HooksDispatcherOnRerender; - children = Component(props, secondArg); + children = __DEV__ + ? callComponentInDEV(Component, props, secondArg) + : Component(props, secondArg); } while (didScheduleRenderPhaseUpdateDuringThisPass); return children; } diff --git a/packages/react-reconciler/src/ReactFiberOwnerStack.js b/packages/react-reconciler/src/ReactFiberOwnerStack.js new file mode 100644 index 000000000000..b8510ebf6bce --- /dev/null +++ b/packages/react-reconciler/src/ReactFiberOwnerStack.js @@ -0,0 +1,112 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import {REACT_LAZY_TYPE} from 'shared/ReactSymbols'; + +import { + callLazyInitInDEV, + callComponentInDEV, + callRenderInDEV, +} from './ReactFiberCallUserSpace'; + +// TODO: Make this configurable on the root. +const externalRegExp = /\/node\_modules\/|\(\\)/; + +let callComponentFrame: null | string = null; +let callIteratorFrame: null | string = null; +let callLazyInitFrame: null | string = null; + +function isNotExternal(stackFrame: string): boolean { + return !externalRegExp.test(stackFrame); +} + +function initCallComponentFrame(): string { + // Extract the stack frame of the callComponentInDEV function. + const error = callComponentInDEV(Error, 'react-stack-top-frame', {}); + const stack = error.stack; + const startIdx = stack.startsWith('Error: react-stack-top-frame\n') ? 29 : 0; + const endIdx = stack.indexOf('\n', startIdx); + if (endIdx === -1) { + return stack.slice(startIdx); + } + return stack.slice(startIdx, endIdx); +} + +function initCallRenderFrame(): string { + // Extract the stack frame of the callRenderInDEV function. + try { + (callRenderInDEV: any)({render: null}); + return ''; + } catch (error) { + const stack = error.stack; + const startIdx = stack.startsWith('TypeError: ') + ? stack.indexOf('\n') + 1 + : 0; + const endIdx = stack.indexOf('\n', startIdx); + if (endIdx === -1) { + return stack.slice(startIdx); + } + return stack.slice(startIdx, endIdx); + } +} + +function initCallLazyInitFrame(): string { + // Extract the stack frame of the callLazyInitInDEV function. + const error = callLazyInitInDEV({ + $$typeof: REACT_LAZY_TYPE, + _init: Error, + _payload: 'react-stack-top-frame', + }); + const stack = error.stack; + const startIdx = stack.startsWith('Error: react-stack-top-frame\n') ? 29 : 0; + const endIdx = stack.indexOf('\n', startIdx); + if (endIdx === -1) { + return stack.slice(startIdx); + } + return stack.slice(startIdx, endIdx); +} + +function filterDebugStack(error: Error): string { + // Since stacks can be quite large and we pass a lot of them, we filter them out eagerly + // to save bandwidth even in DEV. We'll also replay these stacks on the client so by + // stripping them early we avoid that overhead. Otherwise we'd normally just rely on + // the DevTools or framework's ignore lists to filter them out. + let stack = error.stack; + if (stack.startsWith('Error: react-stack-top-frame\n')) { + // V8's default formatting prefixes with the error message which we + // don't want/need. + stack = stack.slice(29); + } + const frames = stack.split('\n').slice(1); + if (callComponentFrame === null) { + callComponentFrame = initCallComponentFrame(); + } + let lastFrameIdx = frames.indexOf(callComponentFrame); + if (lastFrameIdx === -1) { + if (callLazyInitFrame === null) { + callLazyInitFrame = initCallLazyInitFrame(); + } + lastFrameIdx = frames.indexOf(callLazyInitFrame); + if (lastFrameIdx === -1) { + if (callIteratorFrame === null) { + callIteratorFrame = initCallRenderFrame(); + } + lastFrameIdx = frames.indexOf(callIteratorFrame); + } + } + if (lastFrameIdx !== -1) { + // Cut off everything after our "callComponent" slot since it'll be Fiber internals. + frames.length = lastFrameIdx; + } + return frames.filter(isNotExternal).join('\n'); +} + +export function formatOwnerStack(ownerStackTrace: Error): string { + return filterDebugStack(ownerStackTrace); +} diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 6fa9142e2d5e..8e58b5c44aa8 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -195,6 +195,8 @@ export type Fiber = { _debugInfo?: ReactDebugInfo | null, _debugOwner?: ReactComponentInfo | Fiber | null, + _debugStack?: string | Error | null, + _debugTask?: ConsoleTask | null, _debugIsCurrentlyTiming?: boolean, _debugNeedsRemount?: boolean, diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 476838776f32..8cc5c15a9e08 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -716,6 +716,7 @@ describe('ReactHooks', () => { useImperativeHandle(ref, () => {}, props.deps); return null; }); + App.displayName = 'App'; await expect(async () => { await act(() => { @@ -846,6 +847,7 @@ describe('ReactHooks', () => { }); return null; }); + App.displayName = 'App'; await expect(async () => { await act(() => { diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js index 3bd934af38da..8333445c6978 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorLogging-test.js @@ -93,9 +93,11 @@ describe('ReactIncrementalErrorLogging', () => { ), expect.stringMatching( new RegExp( - '\\s+(in|at) ErrorThrowingComponent (.*)\n' + - '\\s+(in|at) span(.*)\n' + - '\\s+(in|at) div(.*)', + gate(flags => flags.enableOwnerStacks) + ? '\\s+(in|at) ErrorThrowingComponent' + : '\\s+(in|at) ErrorThrowingComponent (.*)\n' + + '\\s+(in|at) span(.*)\n' + + '\\s+(in|at) div(.*)', ), ), ); @@ -139,9 +141,11 @@ describe('ReactIncrementalErrorLogging', () => { ), expect.stringMatching( new RegExp( - '\\s+(in|at) ErrorThrowingComponent (.*)\n' + - '\\s+(in|at) span(.*)\n' + - '\\s+(in|at) div(.*)', + gate(flags => flags.enableOwnerStacks) + ? '\\s+(in|at) ErrorThrowingComponent' + : '\\s+(in|at) ErrorThrowingComponent (.*)\n' + + '\\s+(in|at) span(.*)\n' + + '\\s+(in|at) div(.*)', ), ), ); @@ -197,10 +201,12 @@ describe('ReactIncrementalErrorLogging', () => { ), expect.stringMatching( new RegExp( - '\\s+(in|at) ErrorThrowingComponent (.*)\n' + - '\\s+(in|at) span(.*)\n' + - '\\s+(in|at) ErrorBoundary(.*)\n' + - '\\s+(in|at) div(.*)', + gate(flags => flags.enableOwnerStacks) + ? '\\s+(in|at) ErrorThrowingComponent' + : '\\s+(in|at) ErrorThrowingComponent (.*)\n' + + '\\s+(in|at) span(.*)\n' + + '\\s+(in|at) ErrorBoundary(.*)\n' + + '\\s+(in|at) div(.*)', ), ), ); @@ -278,9 +284,7 @@ describe('ReactIncrementalErrorLogging', () => { ), expect.stringMatching( gate(flag => flag.enableOwnerStacks) - ? // With owner stacks the return path is cut off but in this case - // this is also what the owner stack looks like. - new RegExp('\\s+(in|at) Foo (.*)') + ? new RegExp('\\s+(in|at) Foo') : new RegExp( '\\s+(in|at) Foo (.*)\n' + '\\s+(in|at) ErrorBoundary(.*)', ), diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js index b2874a99520f..3ec58b2f705a 100644 --- a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js @@ -228,10 +228,18 @@ describe('ReactLazy', () => { expect(error.message).toMatch('Element type is invalid'); assertLog(['Loading...']); - assertConsoleErrorDev([ - 'Expected the result of a dynamic import() call', - 'Expected the result of a dynamic import() call', - ]); + assertConsoleErrorDev( + [ + 'Expected the result of a dynamic import() call', + 'Expected the result of a dynamic import() call', + ], + gate(flags => flags.enableOwnerStacks) + ? { + // There's no owner + withoutStack: true, + } + : undefined, + ); expect(root).not.toMatchRenderedOutput('Hi'); }); @@ -996,10 +1004,7 @@ describe('ReactLazy', () => { await act(() => resolveFakeImport(Foo)); assertLog(['A', 'B']); }).toErrorDev( - ' in Text (at **)\n' + - // TODO: Because this validates after the div has been mounted, it is part of - // the parent stack but since owner stacks will switch to owners this goes away again. - (gate(flags => flags.enableOwnerStacks) ? ' in div (at **)\n' : '') + + (gate(flags => flags.enableOwnerStacks) ? '' : ' in Text (at **)\n') + ' in Foo (at **)', ); expect(root).toMatchRenderedOutput(
AB
); diff --git a/packages/react-reconciler/src/__tests__/ReactMemo-test.js b/packages/react-reconciler/src/__tests__/ReactMemo-test.js index 4b7497893d6b..db2d2e2aebdc 100644 --- a/packages/react-reconciler/src/__tests__/ReactMemo-test.js +++ b/packages/react-reconciler/src/__tests__/ReactMemo-test.js @@ -602,7 +602,7 @@ describe('memo', () => { 'Each child in a list should have a unique "key" prop. ' + 'See https://react.dev/link/warning-keys for more information.\n' + ' in span (at **)\n' + - ' in p (at **)', + ' in ', ); }); @@ -622,16 +622,16 @@ describe('memo', () => { '\n\nCheck the top-level render call using . It was passed a child from Inner. ' + 'See https://react.dev/link/warning-keys for more information.\n' + ' in span (at **)\n' + - ' in Inner (at **)\n' + - ' in p (at **)', + ' in Inner (at **)' + + (gate(flags => flags.enableOwnerStacks) ? '' : '\n in p (at **)'), ); }); - it('should use the inner displayName in the stack', async () => { + it('should use the inner name in the stack', async () => { const fn = (props, ref) => { return []; }; - fn.displayName = 'Inner'; + Object.defineProperty(fn, 'name', {value: 'Inner'}); const MemoComponent = React.memo(fn); ReactNoop.render(

@@ -645,8 +645,8 @@ describe('memo', () => { '\n\nCheck the top-level render call using . It was passed a child from Inner. ' + 'See https://react.dev/link/warning-keys for more information.\n' + ' in span (at **)\n' + - ' in Inner (at **)\n' + - ' in p (at **)', + ' in Inner (at **)' + + (gate(flags => flags.enableOwnerStacks) ? '' : '\n in p (at **)'), ); }); @@ -667,8 +667,8 @@ describe('memo', () => { '\n\nCheck the top-level render call using . It was passed a child from Outer. ' + 'See https://react.dev/link/warning-keys for more information.\n' + ' in span (at **)\n' + - ' in Outer (at **)\n' + - ' in p (at **)', + ' in Outer (at **)' + + (gate(flags => flags.enableOwnerStacks) ? '' : '\n in p (at **)'), ); }); @@ -676,7 +676,7 @@ describe('memo', () => { const fn = (props, ref) => { return []; }; - fn.displayName = 'Inner'; + Object.defineProperty(fn, 'name', {value: 'Inner'}); const MemoComponent = React.memo(fn); MemoComponent.displayName = 'Outer'; ReactNoop.render( @@ -691,8 +691,8 @@ describe('memo', () => { '\n\nCheck the top-level render call using . It was passed a child from Inner. ' + 'See https://react.dev/link/warning-keys for more information.\n' + ' in span (at **)\n' + - ' in Inner (at **)\n' + - ' in p (at **)', + ' in Inner (at **)' + + (gate(flags => flags.enableOwnerStacks) ? '' : '\n in p (at **)'), ); }); } diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 029671bbda81..6060e3ba6002 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -752,6 +752,7 @@ function getCurrentStackInDEV(): string { if (currentTaskInDEV === null || currentTaskInDEV.componentStack === null) { return ''; } + // TODO: Support owner based stacks for logs during SSR. return getStackByComponentStackNode(currentTaskInDEV.componentStack); } return ''; diff --git a/packages/react/src/ReactForwardRef.js b/packages/react/src/ReactForwardRef.js index 3d86241430e3..ad9f5a909036 100644 --- a/packages/react/src/ReactForwardRef.js +++ b/packages/react/src/ReactForwardRef.js @@ -68,7 +68,9 @@ export function forwardRef( // React.forwardRef((props, ref) => {...}); // This kind of inner function is not used elsewhere so the side effect is okay. if (!render.name && !render.displayName) { - render.displayName = name; + Object.defineProperty(render, 'name', { + value: name, + }); } }, }); diff --git a/packages/react/src/ReactMemo.js b/packages/react/src/ReactMemo.js index 91f4864999d3..2948a28193e8 100644 --- a/packages/react/src/ReactMemo.js +++ b/packages/react/src/ReactMemo.js @@ -48,7 +48,9 @@ export function memo( // React.memo((props) => {...}); // This kind of inner function is not used elsewhere so the side effect is okay. if (!type.name && !type.displayName) { - type.displayName = name; + Object.defineProperty(type, 'name', { + value: name, + }); } }, }); diff --git a/packages/react/src/__tests__/ReactCreateRef-test.js b/packages/react/src/__tests__/ReactCreateRef-test.js index 34dc5aa3aa68..616c62e00c88 100644 --- a/packages/react/src/__tests__/ReactCreateRef-test.js +++ b/packages/react/src/__tests__/ReactCreateRef-test.js @@ -45,8 +45,10 @@ describe('ReactCreateRef', () => { ).toErrorDev( 'Unexpected ref object provided for div. ' + 'Use either a ref-setter function or React.createRef().\n' + - ' in div (at **)\n' + - ' in Wrapper (at **)', + ' in div (at **)' + + (gate(flags => flags.enableOwnerStacks) + ? '' + : '\n in Wrapper (at **)'), ); expect(() => @@ -60,8 +62,10 @@ describe('ReactCreateRef', () => { ).toErrorDev( 'Unexpected ref object provided for ExampleComponent. ' + 'Use either a ref-setter function or React.createRef().\n' + - ' in ExampleComponent (at **)\n' + - ' in Wrapper (at **)', + ' in ExampleComponent (at **)' + + (gate(flags => flags.enableOwnerStacks) + ? '' + : '\n in Wrapper (at **)'), ); }); }); diff --git a/packages/react/src/__tests__/ReactElementValidator-test.internal.js b/packages/react/src/__tests__/ReactElementValidator-test.internal.js index 6d133fc8d9b9..7cac9d7c8390 100644 --- a/packages/react/src/__tests__/ReactElementValidator-test.internal.js +++ b/packages/react/src/__tests__/ReactElementValidator-test.internal.js @@ -142,11 +142,10 @@ describe('ReactElementValidator', () => { '"key" prop.\n\nCheck the render method of `Component`. See ' + 'https://react.dev/link/warning-keys for more information.\n' + ' in div (at **)\n' + - // TODO: Because this validates after the div has been mounted, it is part of - // the parent stack but since owner stacks will switch to owners this goes away again. - (gate(flags => flags.enableOwnerStacks) ? ' in div (at **)\n' : '') + ' in Component (at **)\n' + - ' in Parent (at **)\n' + + (gate(flags => flags.enableOwnerStacks) + ? '' + : ' in Parent (at **)\n') + ' in GrandParent (at **)', ); }); @@ -262,8 +261,6 @@ describe('ReactElementValidator', () => { 'Each child in a list should have a unique "key" prop.' + '\n\nCheck the render method of `ParentComp`. It was passed a child from MyComp. ' + 'See https://react.dev/link/warning-keys for more information.\n' + - // TODO: Because this validates after the div has been mounted, it is part of - // the parent stack but since owner stacks will switch to owners this goes away again. ' in div (at **)\n' + ' in MyComp (at **)\n' + ' in ParentComp (at **)', diff --git a/packages/react/src/__tests__/ReactJSXElementValidator-test.js b/packages/react/src/__tests__/ReactJSXElementValidator-test.js index bbafbc74011a..7ac4ead790f6 100644 --- a/packages/react/src/__tests__/ReactJSXElementValidator-test.js +++ b/packages/react/src/__tests__/ReactJSXElementValidator-test.js @@ -209,8 +209,6 @@ describe('ReactJSXElementValidator', () => { 'Each child in a list should have a unique "key" prop.' + '\n\nCheck the render method of `ParentComp`. It was passed a child from MyComp. ' + 'See https://react.dev/link/warning-keys for more information.\n' + - // TODO: Because this validates after the div has been mounted, it is part of - // the parent stack but since owner stacks will switch to owners this goes away again. ' in div (at **)\n' + ' in MyComp (at **)\n' + ' in ParentComp (at **)', diff --git a/packages/react/src/__tests__/ReactJSXRuntime-test.js b/packages/react/src/__tests__/ReactJSXRuntime-test.js index ae26e133fb17..6e6c111589e3 100644 --- a/packages/react/src/__tests__/ReactJSXRuntime-test.js +++ b/packages/react/src/__tests__/ReactJSXRuntime-test.js @@ -299,10 +299,9 @@ describe('ReactJSXRuntime', () => { }).toErrorDev( 'Warning: Each child in a list should have a unique "key" prop.\n\n' + 'Check the render method of `Parent`. See https://react.dev/link/warning-keys for more information.\n' + - ' in Child (at **)\n' + - // TODO: Because this validates after the div has been mounted, it is part of - // the parent stack but since owner stacks will switch to owners this goes away again. - (gate(flags => flags.enableOwnerStacks) ? ' in div (at **)\n' : '') + + (gate(flags => flags.enableOwnerStacks) + ? '' + : ' in Child (at **)\n') + ' in Parent (at **)', ); }); diff --git a/packages/react/src/__tests__/createReactClassIntegration-test.js b/packages/react/src/__tests__/createReactClassIntegration-test.js index 0b65b1425781..cfdc7174c7ba 100644 --- a/packages/react/src/__tests__/createReactClassIntegration-test.js +++ b/packages/react/src/__tests__/createReactClassIntegration-test.js @@ -594,6 +594,7 @@ describe('create-react-class-integration', () => { return null; }, }); + Component.displayName = 'Component'; await expect(async () => { await expect(async () => { @@ -643,6 +644,7 @@ describe('create-react-class-integration', () => { return null; }, }); + Component.displayName = 'Component'; await expect(async () => { await expect(async () => { diff --git a/packages/react/src/__tests__/forwardRef-test.js b/packages/react/src/__tests__/forwardRef-test.js index 96b0ee96d2b1..fdf6d20ae0aa 100644 --- a/packages/react/src/__tests__/forwardRef-test.js +++ b/packages/react/src/__tests__/forwardRef-test.js @@ -197,7 +197,7 @@ describe('forwardRef', () => { '\n\nCheck the top-level render call using . It was passed a child from ForwardRef. ' + 'See https://react.dev/link/warning-keys for more information.\n' + ' in span (at **)\n' + - ' in p (at **)', + ' in ', ); }); @@ -217,16 +217,16 @@ describe('forwardRef', () => { '\n\nCheck the top-level render call using . It was passed a child from ForwardRef(Inner). ' + 'See https://react.dev/link/warning-keys for more information.\n' + ' in span (at **)\n' + - ' in Inner (at **)\n' + - ' in p (at **)', + ' in Inner (at **)' + + (gate(flags => flags.enableOwnerStacks) ? '' : '\n in p (at **)'), ); }); - it('should use the inner displayName in the stack', async () => { + it('should use the inner name in the stack', async () => { const fn = (props, ref) => { return []; }; - fn.displayName = 'Inner'; + Object.defineProperty(fn, 'name', {value: 'Inner'}); const RefForwardingComponent = React.forwardRef(fn); ReactNoop.render(

@@ -240,8 +240,8 @@ describe('forwardRef', () => { '\n\nCheck the top-level render call using . It was passed a child from ForwardRef(Inner). ' + 'See https://react.dev/link/warning-keys for more information.\n' + ' in span (at **)\n' + - ' in Inner (at **)\n' + - ' in p (at **)', + ' in Inner (at **)' + + (gate(flags => flags.enableOwnerStacks) ? '' : '\n in p (at **)'), ); }); @@ -262,16 +262,16 @@ describe('forwardRef', () => { '\n\nCheck the top-level render call using . It was passed a child from Outer. ' + 'See https://react.dev/link/warning-keys for more information.\n' + ' in span (at **)\n' + - ' in Outer (at **)\n' + - ' in p (at **)', + ' in Outer (at **)' + + (gate(flags => flags.enableOwnerStacks) ? '' : '\n in p (at **)'), ); }); - it('should prefer the inner to the outer displayName in the stack', async () => { + it('should prefer the inner name to the outer displayName in the stack', async () => { const fn = (props, ref) => { return []; }; - fn.displayName = 'Inner'; + Object.defineProperty(fn, 'name', {value: 'Inner'}); const RefForwardingComponent = React.forwardRef(fn); RefForwardingComponent.displayName = 'Outer'; ReactNoop.render( @@ -286,8 +286,8 @@ describe('forwardRef', () => { '\n\nCheck the top-level render call using . It was passed a child from Outer. ' + 'See https://react.dev/link/warning-keys for more information.\n' + ' in span (at **)\n' + - ' in Inner (at **)\n' + - ' in p (at **)', + ' in Inner (at **)' + + (gate(flags => flags.enableOwnerStacks) ? '' : '\n in p (at **)'), ); }); diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js index f76a84f899ab..a5a9880b05b9 100644 --- a/packages/react/src/jsx/ReactJSXElement.js +++ b/packages/react/src/jsx/ReactJSXElement.js @@ -492,7 +492,16 @@ export function jsxProdSignatureRunningInDevWithDynamicChildren( ) { if (__DEV__) { const isStaticChildren = false; - return jsxDEV(type, config, maybeKey, isStaticChildren, source, self); + return jsxDEVImpl( + type, + config, + maybeKey, + isStaticChildren, + source, + self, + __DEV__ && enableOwnerStacks ? Error('react-stack-top-frame') : undefined, + __DEV__ && enableOwnerStacks ? createTask(getTaskName(type)) : undefined, + ); } } @@ -505,7 +514,16 @@ export function jsxProdSignatureRunningInDevWithStaticChildren( ) { if (__DEV__) { const isStaticChildren = true; - return jsxDEV(type, config, maybeKey, isStaticChildren, source, self); + return jsxDEVImpl( + type, + config, + maybeKey, + isStaticChildren, + source, + self, + __DEV__ && enableOwnerStacks ? Error('react-stack-top-frame') : undefined, + __DEV__ && enableOwnerStacks ? createTask(getTaskName(type)) : undefined, + ); } } @@ -518,6 +536,28 @@ const didWarnAboutKeySpread = {}; * @param {string} key */ export function jsxDEV(type, config, maybeKey, isStaticChildren, source, self) { + return jsxDEVImpl( + type, + config, + maybeKey, + isStaticChildren, + source, + self, + __DEV__ && enableOwnerStacks ? Error('react-stack-top-frame') : undefined, + __DEV__ && enableOwnerStacks ? createTask(getTaskName(type)) : undefined, + ); +} + +function jsxDEVImpl( + type, + config, + maybeKey, + isStaticChildren, + source, + self, + debugStack, + debugTask, +) { if (__DEV__) { if (!isValidElementType(type)) { // This is an invalid element type. @@ -716,8 +756,8 @@ export function jsxDEV(type, config, maybeKey, isStaticChildren, source, self) { source, getOwner(), props, - __DEV__ && enableOwnerStacks ? Error('react-stack-top-frame') : undefined, - __DEV__ && enableOwnerStacks ? createTask(getTaskName(type)) : undefined, + debugStack, + debugTask, ); } } diff --git a/packages/shared/ReactElementType.js b/packages/shared/ReactElementType.js index 6136f1ac651a..6860932176ee 100644 --- a/packages/shared/ReactElementType.js +++ b/packages/shared/ReactElementType.js @@ -9,10 +9,6 @@ import type {ReactDebugInfo} from './ReactTypes'; -interface ConsoleTask { - run(f: () => T): T; -} - export type ReactElement = { $$typeof: any, type: any, diff --git a/scripts/flow/environment.js b/scripts/flow/environment.js index 88683e7ee86c..f71e5bba2a1e 100644 --- a/scripts/flow/environment.js +++ b/scripts/flow/environment.js @@ -29,6 +29,10 @@ declare module 'create-react-class' { declare const exports: React$CreateClass; } +declare interface ConsoleTask { + run(f: () => T): T; +} + // Flow hides the props of React$Element, this overrides it to unhide // them for React internals. // prettier-ignore diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js index 50267cc2010c..1114b65dfa82 100644 --- a/scripts/jest/matchers/toWarnDev.js +++ b/scripts/jest/matchers/toWarnDev.js @@ -16,6 +16,11 @@ function normalizeCodeLocInfo(str) { // React format: // in Component (at filename.js:123) return str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function (m, name) { + if (name.endsWith('.render')) { + // Class components will have the `render` method as part of their stack trace. + // We strip that out in our normalization to make it look more like component stacks. + name = name.slice(0, name.length - 7); + } return '\n in ' + name + ' (at **)'; }); }