From 91a461af7ad50e229bce66bde55fb4dd207d2bd6 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 22 Jul 2024 23:42:13 -0400 Subject: [PATCH] Call life-cycles with a react-stack-bottom-frame stack frame Most hooks and such are called inside renders which already have these on the stack but life-cycles that call out on them are useful to cut off too. Typically we don't create JSX in here so they wouldn't be part of owner stacks anyway but they can be apart of plain stacks such as the ones prefixes to console logs or printed by error dialogs. --- .../src/__tests__/console-test.js | 21 ++- .../src/ReactFiberCallUserSpace.js | 125 ++++++++++++++++ .../src/ReactFiberCommitWork.js | 141 ++++++++++++------ 3 files changed, 228 insertions(+), 59 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/console-test.js b/packages/react-devtools-shared/src/__tests__/console-test.js index da1990921dc29..df56bb00b9e1d 100644 --- a/packages/react-devtools-shared/src/__tests__/console-test.js +++ b/packages/react-devtools-shared/src/__tests__/console-test.js @@ -254,12 +254,12 @@ describe('console', () => { ); const Child = ({children}) => { - React.useLayoutEffect(() => { + React.useLayoutEffect(function Child_useLayoutEffect() { fakeConsole.error('active error'); fakeConsole.log('active log'); fakeConsole.warn('active warn'); }); - React.useEffect(() => { + React.useEffect(function Child_useEffect() { fakeConsole.error('passive error'); fakeConsole.log('passive log'); fakeConsole.warn('passive warn'); @@ -279,15 +279,14 @@ describe('console', () => { expect(mockWarn.mock.calls[0][0]).toBe('active warn'); expect(normalizeCodeLocInfo(mockWarn.mock.calls[0][1])).toEqual( supportsOwnerStacks - ? // TODO: It would be nice to have a Child stack frame here since it's just the effect function. - '\n in Parent (at **)' + ? '\n in Child_useLayoutEffect (at **)\n in Parent (at **)' : '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)', ); expect(mockWarn.mock.calls[1]).toHaveLength(2); expect(mockWarn.mock.calls[1][0]).toBe('passive warn'); expect(normalizeCodeLocInfo(mockWarn.mock.calls[1][1])).toEqual( supportsOwnerStacks - ? '\n in Parent (at **)' + ? '\n in Child_useEffect (at **)\n in Parent (at **)' : '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)', ); expect(mockError).toHaveBeenCalledTimes(2); @@ -295,14 +294,14 @@ describe('console', () => { expect(mockError.mock.calls[0][0]).toBe('active error'); expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toBe( supportsOwnerStacks - ? '\n in Parent (at **)' + ? '\n in Child_useLayoutEffect (at **)\n in Parent (at **)' : '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)', ); expect(mockError.mock.calls[1]).toHaveLength(2); expect(mockError.mock.calls[1][0]).toBe('passive error'); expect(normalizeCodeLocInfo(mockError.mock.calls[1][1])).toBe( supportsOwnerStacks - ? '\n in Parent (at **)' + ? '\n in Child_useEffect (at **)\n in Parent (at **)' : '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)', ); }); @@ -346,14 +345,14 @@ describe('console', () => { expect(mockWarn.mock.calls[0][0]).toBe('didMount warn'); expect(normalizeCodeLocInfo(mockWarn.mock.calls[0][1])).toEqual( supportsOwnerStacks - ? '\n in Parent (at **)' + ? '\n in Child.componentDidMount (at **)\n in Parent (at **)' : '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)', ); expect(mockWarn.mock.calls[1]).toHaveLength(2); expect(mockWarn.mock.calls[1][0]).toBe('didUpdate warn'); expect(normalizeCodeLocInfo(mockWarn.mock.calls[1][1])).toEqual( supportsOwnerStacks - ? '\n in Parent (at **)' + ? '\n in Child.componentDidUpdate (at **)\n in Parent (at **)' : '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)', ); expect(mockError).toHaveBeenCalledTimes(2); @@ -361,14 +360,14 @@ describe('console', () => { expect(mockError.mock.calls[0][0]).toBe('didMount error'); expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toBe( supportsOwnerStacks - ? '\n in Parent (at **)' + ? '\n in Child.componentDidMount (at **)\n in Parent (at **)' : '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)', ); expect(mockError.mock.calls[1]).toHaveLength(2); expect(mockError.mock.calls[1][0]).toBe('didUpdate error'); expect(normalizeCodeLocInfo(mockError.mock.calls[1][1])).toBe( supportsOwnerStacks - ? '\n in Parent (at **)' + ? '\n in Child.componentDidUpdate (at **)\n in Parent (at **)' : '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)', ); }); diff --git a/packages/react-reconciler/src/ReactFiberCallUserSpace.js b/packages/react-reconciler/src/ReactFiberCallUserSpace.js index 2012d395ef033..39b8d0543d1be 100644 --- a/packages/react-reconciler/src/ReactFiberCallUserSpace.js +++ b/packages/react-reconciler/src/ReactFiberCallUserSpace.js @@ -7,9 +7,12 @@ * @flow */ +import type {Fiber} from './ReactInternalTypes'; import type {LazyComponent} from 'react/src/ReactLazy'; +import type {Effect} from './ReactFiberHooks'; import {isRendering, setIsRendering} from './ReactCurrentFiber'; +import {captureCommitPhaseError} from './ReactFiberWorkLoop'; // 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. @@ -42,6 +45,13 @@ export const callComponentInDEV: ( interface ClassInstance { render(): R; + componentDidMount(): void; + componentDidUpdate( + prevProps: Object, + prevState: Object, + snaphot: Object, + ): void; + componentWillUnmount(): void; } const callRender = { @@ -63,6 +73,121 @@ export const callRenderInDEV: (instance: ClassInstance) => R => R = (callRender['react-stack-bottom-frame'].bind(callRender): any) : (null: any); +const callComponentDidMount = { + 'react-stack-bottom-frame': function ( + finishedWork: Fiber, + instance: ClassInstance, + ): void { + try { + instance.componentDidMount(); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + }, +}; + +export const callComponentDidMountInDEV: ( + finishedWork: Fiber, + instance: ClassInstance, +) => void = __DEV__ + ? // We use this technique to trick minifiers to preserve the function name. + (callComponentDidMount['react-stack-bottom-frame'].bind( + callComponentDidMount, + ): any) + : (null: any); + +const callComponentDidUpdate = { + 'react-stack-bottom-frame': function ( + finishedWork: Fiber, + instance: ClassInstance, + prevProps: Object, + prevState: Object, + snapshot: Object, + ): void { + try { + instance.componentDidUpdate(prevProps, prevState, snapshot); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } + }, +}; + +export const callComponentDidUpdateInDEV: ( + finishedWork: Fiber, + instance: ClassInstance, + prevProps: Object, + prevState: Object, + snaphot: Object, +) => void = __DEV__ + ? // We use this technique to trick minifiers to preserve the function name. + (callComponentDidUpdate['react-stack-bottom-frame'].bind( + callComponentDidUpdate, + ): any) + : (null: any); + +const callComponentWillUnmount = { + 'react-stack-bottom-frame': function ( + current: Fiber, + nearestMountedAncestor: Fiber | null, + instance: ClassInstance, + ): void { + try { + instance.componentWillUnmount(); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } + }, +}; + +export const callComponentWillUnmountInDEV: ( + current: Fiber, + nearestMountedAncestor: Fiber | null, + instance: ClassInstance, +) => void = __DEV__ + ? // We use this technique to trick minifiers to preserve the function name. + (callComponentWillUnmount['react-stack-bottom-frame'].bind( + callComponentWillUnmount, + ): any) + : (null: any); + +const callCreate = { + 'react-stack-bottom-frame': function (effect: Effect): (() => void) | void { + const create = effect.create; + const inst = effect.inst; + const destroy = create(); + inst.destroy = destroy; + return destroy; + }, +}; + +export const callCreateInDEV: (effect: Effect) => (() => void) | void = __DEV__ + ? // We use this technique to trick minifiers to preserve the function name. + (callCreate['react-stack-bottom-frame'].bind(callCreate): any) + : (null: any); + +const callDestroy = { + 'react-stack-bottom-frame': function ( + current: Fiber, + nearestMountedAncestor: Fiber | null, + destroy: () => void, + ): void { + try { + destroy(); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } + }, +}; + +export const callDestroyInDEV: ( + current: Fiber, + nearestMountedAncestor: Fiber | null, + destroy: () => void, +) => void = __DEV__ + ? // We use this technique to trick minifiers to preserve the function name. + (callDestroy['react-stack-bottom-frame'].bind(callDestroy): any) + : (null: any); + const callLazyInit = { 'react-stack-bottom-frame': function (lazy: LazyComponent): any { const payload = lazy._payload; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index a8d37b5e44936..773e5d8f485d9 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -213,6 +213,13 @@ import { } from './ReactFiberTracingMarkerComponent'; import {scheduleUpdateOnFiber} from './ReactFiberWorkLoop'; import {enqueueConcurrentRenderForLane} from './ReactFiberConcurrentUpdates'; +import { + callComponentDidMountInDEV, + callComponentDidUpdateInDEV, + callComponentWillUnmountInDEV, + callCreateInDEV, + callDestroyInDEV, +} from './ReactFiberCallUserSpace'; let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; if (__DEV__) { @@ -244,7 +251,12 @@ function shouldProfile(current: Fiber): boolean { ); } -function callComponentWillUnmountWithTimer(current: Fiber, instance: any) { +// Capture errors so they don't interrupt unmounting. +function safelyCallComponentWillUnmount( + current: Fiber, + nearestMountedAncestor: Fiber | null, + instance: any, +) { instance.props = resolveClassComponentProps( current.type, current.memoizedProps, @@ -252,27 +264,27 @@ function callComponentWillUnmountWithTimer(current: Fiber, instance: any) { ); instance.state = current.memoizedState; if (shouldProfile(current)) { - try { - startLayoutEffectTimer(); - instance.componentWillUnmount(); - } finally { - recordLayoutEffectDuration(current); + startLayoutEffectTimer(); + if (__DEV__) { + callComponentWillUnmountInDEV(current, nearestMountedAncestor, instance); + } else { + try { + instance.componentWillUnmount(); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } } + recordLayoutEffectDuration(current); } else { - instance.componentWillUnmount(); - } -} - -// Capture errors so they don't interrupt unmounting. -function safelyCallComponentWillUnmount( - current: Fiber, - nearestMountedAncestor: Fiber | null, - instance: any, -) { - try { - callComponentWillUnmountWithTimer(current, instance); - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); + if (__DEV__) { + callComponentWillUnmountInDEV(current, nearestMountedAncestor, instance); + } else { + try { + instance.componentWillUnmount(); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } + } } } @@ -339,10 +351,14 @@ function safelyCallDestroy( nearestMountedAncestor: Fiber | null, destroy: () => void, ) { - try { - destroy(); - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); + if (__DEV__) { + callDestroyInDEV(current, nearestMountedAncestor, destroy); + } else { + try { + destroy(); + } catch (error) { + captureCommitPhaseError(current, nearestMountedAncestor, error); + } } } @@ -626,19 +642,20 @@ function commitHookEffectListMount(flags: HookFlags, finishedWork: Fiber) { } // Mount - const create = effect.create; + let destroy; if (__DEV__) { if ((flags & HookInsertion) !== NoHookEffect) { setIsRunningInsertionEffect(true); } - } - const inst = effect.inst; - const destroy = create(); - inst.destroy = destroy; - if (__DEV__) { + destroy = callCreateInDEV(effect); if ((flags & HookInsertion) !== NoHookEffect) { setIsRunningInsertionEffect(false); } + } else { + const create = effect.create; + const inst = effect.inst; + destroy = create(); + inst.destroy = destroy; } if (enableSchedulingProfiler) { @@ -826,18 +843,26 @@ function commitClassLayoutLifecycles( } } if (shouldProfile(finishedWork)) { - try { - startLayoutEffectTimer(); - instance.componentDidMount(); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); + startLayoutEffectTimer(); + if (__DEV__) { + callComponentDidMountInDEV(finishedWork, instance); + } else { + try { + instance.componentDidMount(); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } } recordLayoutEffectDuration(finishedWork); } else { - try { - instance.componentDidMount(); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); + if (__DEV__) { + callComponentDidMountInDEV(finishedWork, instance); + } else { + try { + instance.componentDidMount(); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } } } } else { @@ -879,26 +904,46 @@ function commitClassLayoutLifecycles( } } if (shouldProfile(finishedWork)) { - try { - startLayoutEffectTimer(); - instance.componentDidUpdate( + startLayoutEffectTimer(); + if (__DEV__) { + callComponentDidUpdateInDEV( + finishedWork, + instance, prevProps, prevState, instance.__reactInternalSnapshotBeforeUpdate, ); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); + } else { + try { + instance.componentDidUpdate( + prevProps, + prevState, + instance.__reactInternalSnapshotBeforeUpdate, + ); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } } recordLayoutEffectDuration(finishedWork); } else { - try { - instance.componentDidUpdate( + if (__DEV__) { + callComponentDidUpdateInDEV( + finishedWork, + instance, prevProps, prevState, instance.__reactInternalSnapshotBeforeUpdate, ); - } catch (error) { - captureCommitPhaseError(finishedWork, finishedWork.return, error); + } else { + try { + instance.componentDidUpdate( + prevProps, + prevState, + instance.__reactInternalSnapshotBeforeUpdate, + ); + } catch (error) { + captureCommitPhaseError(finishedWork, finishedWork.return, error); + } } } }