Repository: facebook/react
Pull request: [Fiber] Call life-cycles with a react-stack-bottom-frame stack frame

Stacked on #30427.

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.

This lets us cut off any React internals below. This should really be possible using just ignore listing too ideally.

At this point we should maybe just build a Babel plugin that lets us annotate a function to need to have this name.


Changed files:
filename: packages/react-devtools-shared/src/__tests__/console-test.js
patch: @@ -254,12 +254,12 @@ describe('console', () => {
       </Intermediate>
     );
     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,30 +279,29 @@ 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);
     expect(mockError.mock.calls[0]).toHaveLength(2);
     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,29 +345,29 @@ 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);
     expect(mockError.mock.calls[0]).toHaveLength(2);
     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 **)',
     );
   });

filename: packages/react-reconciler/src/ReactFiberCallUserSpace.js
patch: @@ -7,9 +7,13 @@
  * @flow
  */
 
+import type {Fiber} from './ReactInternalTypes';
 import type {LazyComponent} from 'react/src/ReactLazy';
+import type {Effect} from './ReactFiberHooks';
+import type {CapturedValue} from './ReactCapturedValue';
 
 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 +46,14 @@ export const callComponentInDEV: <Props, Arg, R>(
 
 interface ClassInstance<R> {
   render(): R;
+  componentDidMount(): void;
+  componentDidUpdate(
+    prevProps: Object,
+    prevState: Object,
+    snaphot: Object,
+  ): void;
+  componentDidCatch(error: mixed, errorInfo: {componentStack: string}): void;
+  componentWillUnmount(): void;
 }
 
 const callRender = {
@@ -63,6 +75,144 @@ export const callRenderInDEV: <R>(instance: ClassInstance<R>) => R => R =
       (callRender['react-stack-bottom-frame'].bind(callRender): any)
     : (null: any);
 
+const callComponentDidMount = {
+  'react-stack-bottom-frame': function (
+    finishedWork: Fiber,
+    instance: ClassInstance<any>,
+  ): void {
+    try {
+      instance.componentDidMount();
+    } catch (error) {
+      captureCommitPhaseError(finishedWork, finishedWork.return, error);
+    }
+  },
+};
+
+export const callComponentDidMountInDEV: (
+  finishedWork: Fiber,
+  instance: ClassInstance<any>,
+) => 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<any>,
+    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<any>,
+  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 callComponentDidCatch = {
+  'react-stack-bottom-frame': function (
+    instance: ClassInstance<any>,
+    errorInfo: CapturedValue<mixed>,
+  ): void {
+    const error = errorInfo.value;
+    const stack = errorInfo.stack;
+    instance.componentDidCatch(error, {
+      componentStack: stack !== null ? stack : '',
+    });
+  },
+};
+
+export const callComponentDidCatchInDEV: (
+  instance: ClassInstance<any>,
+  errorInfo: CapturedValue<mixed>,
+) => void = __DEV__
+  ? // We use this technique to trick minifiers to preserve the function name.
+    (callComponentDidCatch['react-stack-bottom-frame'].bind(
+      callComponentDidCatch,
+    ): any)
+  : (null: any);
+
+const callComponentWillUnmount = {
+  'react-stack-bottom-frame': function (
+    current: Fiber,
+    nearestMountedAncestor: Fiber | null,
+    instance: ClassInstance<any>,
+  ): void {
+    try {
+      instance.componentWillUnmount();
+    } catch (error) {
+      captureCommitPhaseError(current, nearestMountedAncestor, error);
+    }
+  },
+};
+
+export const callComponentWillUnmountInDEV: (
+  current: Fiber,
+  nearestMountedAncestor: Fiber | null,
+  instance: ClassInstance<any>,
+) => 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, any>): any {
     const payload = lazy._payload;

filename: packages/react-reconciler/src/ReactFiberCommitWork.js
patch: @@ -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<mixed> | null = null;
 if (__DEV__) {
@@ -244,35 +251,40 @@ 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,
     current.elementType === current.type,
   );
   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);
+        }
       }
     }
   }

filename: packages/react-reconciler/src/ReactFiberThrow.js
patch: @@ -88,6 +88,7 @@ import {ConcurrentRoot} from './ReactRootTags';
 import {noopSuspenseyCommitThenable} from './ReactFiberThenable';
 import {REACT_POSTPONE_TYPE} from 'shared/ReactSymbols';
 import {runWithFiberInDEV} from './ReactCurrentFiber';
+import {callComponentDidCatchInDEV} from './ReactFiberCallUserSpace';
 
 function createRootErrorUpdate(
   root: FiberRoot,
@@ -172,11 +173,15 @@ function initializeClassErrorUpdate(
         // not defined.
         markLegacyErrorBoundaryAsFailed(this);
       }
-      const error = errorInfo.value;
-      const stack = errorInfo.stack;
-      this.componentDidCatch(error, {
-        componentStack: stack !== null ? stack : '',
-      });
+      if (__DEV__) {
+        callComponentDidCatchInDEV(this, errorInfo);
+      } else {
+        const error = errorInfo.value;
+        const stack = errorInfo.stack;
+        this.componentDidCatch(error, {
+          componentStack: stack !== null ? stack : '',
+        });
+      }
       if (__DEV__) {
         if (typeof getDerivedStateFromError !== 'function') {
           // If componentDidCatch is the only error boundary method defined,