From d46e418d1fda00a4a88137f4f74cdc27d61fdcaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 10 Nov 2020 19:35:27 -0500 Subject: [PATCH] Encode throwing server components as lazy throwing references (#20217) This ensures that if this server component was the child of a client component that has an error boundary, it doesn't trigger the error until this gets rendered so it happens as deep as possible. --- .../src/__tests__/ReactFlight-test.js | 64 +++++++++++++++++++ .../react-server/src/ReactFlightServer.js | 9 ++- 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 838db8337121e..3d4ccf5c459f1 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -16,6 +16,7 @@ let ReactNoop; let ReactNoopFlightServer; let ReactNoopFlightClient; let ErrorBoundary; +let NoErrorExpected; describe('ReactFlight', () => { beforeEach(() => { @@ -47,6 +48,26 @@ describe('ReactFlight', () => { return this.props.children; } }; + + NoErrorExpected = class extends React.Component { + state = {hasError: false, error: null}; + static getDerivedStateFromError(error) { + return { + hasError: true, + error, + }; + } + componentDidMount() { + expect(this.state.error).toBe(null); + expect(this.state.hasError).toBe(false); + } + render() { + if (this.state.hasError) { + return this.state.error.message; + } + return this.props.children; + } + }; }); function moduleReference(value) { @@ -164,6 +185,49 @@ describe('ReactFlight', () => { }); }); + it('should trigger the inner most error boundary inside a client component', () => { + function ServerComponent() { + throw new Error('This was thrown in the server component.'); + } + + function ClientComponent({children}) { + // This should catch the error thrown by the server component, even though it has already happened. + // We currently need to wrap it in a div because as it's set up right now, a lazy reference will + // throw during reconciliation which will trigger the parent of the error boundary. + // This is similar to how these will suspend the parent if it's a direct child of a Suspense boundary. + // That's a bug. + return ( + +
{children}
+
+ ); + } + + const ClientComponentReference = moduleReference(ClientComponent); + + function Server() { + return ( + + + + ); + } + + const data = ReactNoopFlightServer.render(); + + function Client({transport}) { + return ReactNoopFlightClient.read(transport); + } + + act(() => { + ReactNoop.render( + + + , + ); + }); + }); + it('should warn in DEV if a toJSON instance is passed to a host component', () => { expect(() => { const transport = ReactNoopFlightServer.render( diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 76690a192eca8..56842b7b2f40c 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -409,8 +409,13 @@ export function resolveModelToJSON( x.then(ping, ping); return serializeByRefID(newSegment.id); } else { - // Something errored. Don't bother encoding anything up to here. - throw x; + // Something errored. We'll still send everything we have up until this point. + // We'll replace this element with a lazy reference that throws on the client + // once it gets rendered. + request.pendingChunks++; + const errorId = request.nextChunkId++; + emitErrorChunk(request, errorId, x); + return serializeByRefID(errorId); } } }