[Fizz] Refactor Component Stack Nodes (#30298)

Component stacks have a similar problem to the problem with keyPath
where we had to move it down and set it late right before recursing.
Currently we work around that by popping exactly one off when something
suspends. That doesn't work with the new server stacks being added which
are more than one. It also meant that we kept having add a single frame
that could be popped when there shouldn't need to be one.

Unlike keyPath component stacks has this weird property that once
something throws we might need the stack that was attempted for errors
or the previous stack if we're going to retry and just recreate it.

I've tried a few different approaches and I didn't like either but this
is the one that seems least problematic.

I first split out renderNodeDestructive into a retryNode helper. During
retries only retryNode is called. When we first discover a node, we pass
through renderNodeDestructive.

Instead of add a component stack frame deep inside renderNodeDestructive
after we've already refined a node, we now add it before in
renderNodeDestructive. That way it's only added once before being
attempted. This is similar to how Fiber works where in ChildFiber we
match the node once to create the instance and then later do we attempt
to actually render it and it's only the second part that's ever retried.

This unfortunately means that we now have to refine the node down to
element/lazy/thenables twice. To avoid refining the type too I move that
to be done lazily.

Author: sebmarkbage

packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js

@@ -47,6 +47,7 @@ export function describeFiber(
     case HostComponent:
       return describeBuiltInComponentFrame(workInProgress.type);
     case LazyComponent:
+      // TODO: When we support Thenables as component types we should rename this.
       return describeBuiltInComponentFrame('Lazy');
     case SuspenseComponent:
       return describeBuiltInComponentFrame('Suspense');

packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

@@ -700,17 +700,39 @@ describe('ReactDOMFizzServer', () => {
 
   it('should client render a boundary if a lazy component rejects', async () => {
     let rejectComponent;
+    const promise = new Promise((resolve, reject) => {
+      rejectComponent = reject;
+    });
     const LazyComponent = React.lazy(() => {
-      return new Promise((resolve, reject) => {
-        rejectComponent = reject;
-      });
+      return promise;
+    });
+
+    const LazyLazy = React.lazy(async () => {
+      return {
+        default: LazyComponent,
+      };
+    });
+
+    function Wrapper({children}) {
+      return children;
+    }
+    const LazyWrapper = React.lazy(() => {
+      return {
+        then(callback) {
+          callback({
+            default: Wrapper,
+          });
+        },
+      };
     });
 
     function App({isClient}) {
       return (
         <div>
           <Suspense fallback={<Text text="Loading..." />}>
-            {isClient ? <Text text="Hello" /> : <LazyComponent text="Hello" />}
+            <LazyWrapper>
+              {isClient ? <Text text="Hello" /> : <LazyLazy text="Hello" />}
+            </LazyWrapper>
           </Suspense>
         </div>
       );
@@ -744,6 +766,7 @@ describe('ReactDOMFizzServer', () => {
       });
       pipe(writable);
     });
+
     expect(loggedErrors).toEqual([]);
     expect(bootstrapped).toBe(true);
 
@@ -772,7 +795,7 @@ describe('ReactDOMFizzServer', () => {
           'Switched to client rendering because the server rendering errored:\n\n' +
             theError.message,
           expectedDigest,
-          componentStack(['Lazy', 'Suspense', 'div', 'App']),
+          componentStack(['Lazy', 'Wrapper', 'Suspense', 'div', 'App']),
         ],
       ],
       [
@@ -852,13 +875,9 @@ describe('ReactDOMFizzServer', () => {
     }
 
     await act(() => {
-      const {pipe} = renderToPipeableStream(
-        <App isClient={false} />,
-
-        {
-          onError,
-        },
-      );
+      const {pipe} = renderToPipeableStream(<App isClient={false} />, {
+        onError,
+      });
       pipe(writable);
     });
     expect(loggedErrors).toEqual([]);
@@ -896,7 +915,7 @@ describe('ReactDOMFizzServer', () => {
           'Switched to client rendering because the server rendering errored:\n\n' +
             theError.message,
           expectedDigest,
-          componentStack(['Lazy', 'Suspense', 'div', 'App']),
+          componentStack(['Suspense', 'div', 'App']),
         ],
       ],
       [
@@ -1395,13 +1414,13 @@ describe('ReactDOMFizzServer', () => {
             'The render was aborted by the server without a reason.',
           expectedDigest,
           // We get the stack of the task when it was aborted which is why we see `h1`
-          componentStack(['h1', 'Suspense', 'div', 'App']),
+          componentStack(['AsyncText', 'h1', 'Suspense', 'div', 'App']),
         ],
         [
           'Switched to client rendering because the server rendering aborted due to:\n\n' +
             'The render was aborted by the server without a reason.',
           expectedDigest,
-          componentStack(['Suspense', 'main', 'div', 'App']),
+          componentStack(['AsyncText', 'Suspense', 'main', 'div', 'App']),
         ],
       ],
       [
@@ -3523,13 +3542,13 @@ describe('ReactDOMFizzServer', () => {
           'Switched to client rendering because the server rendering aborted due to:\n\n' +
             'foobar',
           'a digest',
-          componentStack(['Suspense', 'p', 'div', 'App']),
+          componentStack(['AsyncText', 'Suspense', 'p', 'div', 'App']),
         ],
         [
           'Switched to client rendering because the server rendering aborted due to:\n\n' +
             'foobar',
           'a digest',
-          componentStack(['Suspense', 'span', 'div', 'App']),
+          componentStack(['AsyncText', 'Suspense', 'span', 'div', 'App']),
         ],
       ],
       [
@@ -3606,13 +3625,13 @@ describe('ReactDOMFizzServer', () => {
           'Switched to client rendering because the server rendering aborted due to:\n\n' +
             'uh oh',
           'a digest',
-          componentStack(['Suspense', 'p', 'div', 'App']),
+          componentStack(['AsyncText', 'Suspense', 'p', 'div', 'App']),
         ],
         [
           'Switched to client rendering because the server rendering aborted due to:\n\n' +
             'uh oh',
           'a digest',
-          componentStack(['Suspense', 'span', 'div', 'App']),
+          componentStack(['AsyncText', 'Suspense', 'span', 'div', 'App']),
         ],
       ],
       [

packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js

@@ -585,7 +585,7 @@ describe('ReactDOMFizzServerNode', () => {
     let isComplete = false;
     let rendered = false;
     const promise = new Promise(r => (resolve = r));
-    function Wait() {
+    function Wait({prop}) {
       if (!hasLoaded) {
         throw promise;
       }

packages/react-html/src/__tests__/ReactHTMLServer-test.js

@@ -250,9 +250,7 @@ if (!__EXPERIMENTAL__) {
               '\n    in Bar (at **)' +
               '\n    in Foo (at **)' +
               '\n    in div (at **)'
-          : '\n    in Lazy (at **)' +
-              '\n    in div (at **)' +
-              '\n    in div (at **)',
+          : '\n    in div (at **)' + '\n    in div (at **)',
       );
       expect(normalizeCodeLocInfo(caughtErrors[0].ownerStack)).toBe(
         __DEV__ && gate(flags => flags.enableOwnerStacks)

packages/react-reconciler/src/ReactFiberComponentStack.js

@@ -39,6 +39,7 @@ function describeFiber(fiber: Fiber): string {
     case HostComponent:
       return describeBuiltInComponentFrame(fiber.type);
     case LazyComponent:
+      // TODO: When we support Thenables as component types we should rename this.
       return describeBuiltInComponentFrame('Lazy');
     case SuspenseComponent:
       return describeBuiltInComponentFrame('Suspense');

packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js

@@ -930,4 +930,74 @@ describe('ReactFlightDOMEdge', () => {
       '\n    in Bar (at **)' + '\n    in Foo (at **)',
     );
   });
+
+  it('supports server components in ssr component stacks', async () => {
+    let reject;
+    const promise = new Promise((_, r) => (reject = r));
+    async function Erroring() {
+      await promise;
+      return 'should not render';
+    }
+
+    const model = {
+      root: ReactServer.createElement(Erroring),
+    };
+
+    const stream = ReactServerDOMServer.renderToReadableStream(
+      model,
+      webpackMap,
+      {
+        onError() {},
+      },
+    );
+
+    const rootModel = await ReactServerDOMClient.createFromReadableStream(
+      stream,
+      {
+        ssrManifest: {
+          moduleMap: null,
+          moduleLoading: null,
+        },
+      },
+    );
+
+    const errors = [];
+    const result = ReactDOMServer.renderToReadableStream(
+      <div>{rootModel.root}</div>,
+      {
+        onError(error, {componentStack}) {
+          errors.push({
+            error,
+            componentStack: normalizeCodeLocInfo(componentStack),
+          });
+        },
+      },
+    );
+
+    const theError = new Error('my error');
+    reject(theError);
+
+    const expectedMessage = __DEV__
+      ? 'my error'
+      : 'An error occurred in the Server Components render. The specific message is omitted in production builds to avoid leaking sensitive details. A digest property is included on this error instance which may provide additional details about the nature of the error.';
+
+    try {
+      await result;
+    } catch (x) {
+      expect(x).toEqual(
+        expect.objectContaining({
+          message: expectedMessage,
+        }),
+      );
+    }
+
+    expect(errors).toEqual([
+      {
+        error: expect.objectContaining({
+          message: expectedMessage,
+        }),
+        componentStack: (__DEV__ ? '\n    in Erroring' : '') + '\n    in div',
+      },
+    ]);
+  });
 });