diff --git a/.eslintrc.js b/.eslintrc.js index 05a6ecae3ade8..47ea90f62274f 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -571,6 +571,7 @@ module.exports = { TimeoutID: 'readonly', WheelEventHandler: 'readonly', FinalizationRegistry: 'readonly', + Omit: 'readonly', spyOnDev: 'readonly', spyOnDevAndProd: 'readonly', diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 086a3ae46b802..e1d43db1ecffa 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -72,6 +72,8 @@ import { import getComponentNameFromType from 'shared/getComponentNameFromType'; +import isArray from 'shared/isArray'; + export type {CallServerCallback, EncodeFormActionCallback}; interface FlightStreamController { @@ -101,7 +103,6 @@ type RowParserState = 0 | 1 | 2 | 3 | 4; const PENDING = 'pending'; const BLOCKED = 'blocked'; -const CYCLIC = 'cyclic'; const RESOLVED_MODEL = 'resolved_model'; const RESOLVED_MODULE = 'resolved_module'; const INITIALIZED = 'fulfilled'; @@ -123,14 +124,6 @@ type BlockedChunk = { _debugInfo?: null | ReactDebugInfo, then(resolve: (T) => mixed, reject?: (mixed) => mixed): void, }; -type CyclicChunk = { - status: 'cyclic', - value: null | Array<(T) => mixed>, - reason: null | Array<(mixed) => mixed>, - _response: Response, - _debugInfo?: null | ReactDebugInfo, - then(resolve: (T) => mixed, reject?: (mixed) => mixed): void, -}; type ResolvedModelChunk = { status: 'resolved_model', value: UninitializedModel, @@ -176,7 +169,6 @@ type ErroredChunk = { type SomeChunk = | PendingChunk | BlockedChunk - | CyclicChunk | ResolvedModelChunk | ResolvedModuleChunk | InitializedChunk @@ -218,7 +210,6 @@ Chunk.prototype.then = function ( break; case PENDING: case BLOCKED: - case CYCLIC: if (resolve) { if (chunk.value === null) { chunk.value = ([]: Array<(T) => mixed>); @@ -278,7 +269,6 @@ function readChunk(chunk: SomeChunk): T { return chunk.value; case PENDING: case BLOCKED: - case CYCLIC: // eslint-disable-next-line no-throw-literal throw ((chunk: any): Thenable); default: @@ -327,7 +317,6 @@ function wakeChunkIfInitialized( break; case PENDING: case BLOCKED: - case CYCLIC: if (chunk.value) { for (let i = 0; i < resolveListeners.length; i++) { chunk.value.push(resolveListeners[i]); @@ -501,51 +490,61 @@ function resolveModuleChunk( } } -let initializingChunk: ResolvedModelChunk = (null: any); -let initializingChunkBlockedModel: null | {deps: number, value: any} = null; +type InitializationHandler = { + parent: null | InitializationHandler, + chunk: null | BlockedChunk, + value: any, + deps: number, + errored: boolean, +}; +let initializingHandler: null | InitializationHandler = null; + function initializeModelChunk(chunk: ResolvedModelChunk): void { - const prevChunk = initializingChunk; - const prevBlocked = initializingChunkBlockedModel; - initializingChunk = chunk; - initializingChunkBlockedModel = null; + const prevHandler = initializingHandler; + initializingHandler = null; const resolvedModel = chunk.value; - // We go to the CYCLIC state until we've fully resolved this. + // We go to the BLOCKED state until we've fully resolved this. // We do this before parsing in case we try to initialize the same chunk // while parsing the model. Such as in a cyclic reference. - const cyclicChunk: CyclicChunk = (chunk: any); - cyclicChunk.status = CYCLIC; + const cyclicChunk: BlockedChunk = (chunk: any); + cyclicChunk.status = BLOCKED; cyclicChunk.value = null; cyclicChunk.reason = null; try { const value: T = parseModel(chunk._response, resolvedModel); - if ( - initializingChunkBlockedModel !== null && - initializingChunkBlockedModel.deps > 0 - ) { - initializingChunkBlockedModel.value = value; - // We discovered new dependencies on modules that are not yet resolved. - // We have to go the BLOCKED state until they're resolved. - const blockedChunk: BlockedChunk = (chunk: any); - blockedChunk.status = BLOCKED; - } else { - const resolveListeners = cyclicChunk.value; - const initializedChunk: InitializedChunk = (chunk: any); - initializedChunk.status = INITIALIZED; - initializedChunk.value = value; - if (resolveListeners !== null) { - wakeChunk(resolveListeners, value); + // Invoke any listeners added while resolving this model. I.e. cyclic + // references. This may or may not fully resolve the model depending on + // if they were blocked. + const resolveListeners = cyclicChunk.value; + if (resolveListeners !== null) { + cyclicChunk.value = null; + cyclicChunk.reason = null; + wakeChunk(resolveListeners, value); + } + if (initializingHandler !== null) { + if (initializingHandler.errored) { + throw initializingHandler.value; + } + if (initializingHandler.deps > 0) { + // We discovered new dependencies on modules that are not yet resolved. + // We have to keep the BLOCKED state until they're resolved. + initializingHandler.value = value; + initializingHandler.chunk = cyclicChunk; + return; } } + const initializedChunk: InitializedChunk = (chunk: any); + initializedChunk.status = INITIALIZED; + initializedChunk.value = value; } catch (error) { const erroredChunk: ErroredChunk = (chunk: any); erroredChunk.status = ERRORED; erroredChunk.reason = error; } finally { - initializingChunk = prevChunk; - initializingChunkBlockedModel = prevBlocked; + initializingHandler = prevHandler; } } @@ -626,7 +625,9 @@ function createElement( owner: null | ReactComponentInfo, // DEV-only stack: null | string, // DEV-only validated: number, // DEV-only -): React$Element { +): + | React$Element + | LazyComponent, SomeChunk>> { let element: any; if (__DEV__ && enableRefAsProp) { // `ref` is non-enumerable in dev @@ -723,15 +724,60 @@ function createElement( value: task, }); } + } + + if (initializingHandler !== null) { + const handler = initializingHandler; + // We pop the stack to the previous outer handler before leaving the Element. + // This is effectively the complete phase. + initializingHandler = handler.parent; + if (handler.errored) { + // Something errored inside this Element's props. We can turn this Element + // into a Lazy so that we can still render up until that Lazy is rendered. + const erroredChunk: ErroredChunk> = createErrorChunk( + response, + handler.value, + ); + if (__DEV__) { + // Conceptually the error happened inside this Element but right before + // it was rendered. We don't have a client side component to render but + // we can add some DebugInfo to explain that this was conceptually a + // Server side error that errored inside this element. That way any stack + // traces will point to the nearest JSX that errored - e.g. during + // serialization. + const erroredComponent: ReactComponentInfo = { + name: getComponentNameFromType(element.type) || '', + owner: element._owner, + }; + if (enableOwnerStacks) { + // $FlowFixMe[cannot-write] + erroredComponent.stack = element._debugStack; + // $FlowFixMe[cannot-write] + erroredComponent.task = element._debugTask; + } + erroredChunk._debugInfo = [erroredComponent]; + } + return createLazyChunkWrapper(erroredChunk); + } + if (handler.deps > 0) { + // We have blocked references inside this Element but we can turn this into + // a Lazy node referencing this Element to let everything around it proceed. + const blockedChunk: BlockedChunk> = + createBlockedChunk(response); + handler.value = element; + handler.chunk = blockedChunk; + if (__DEV__) { + const freeze = Object.freeze.bind(Object, element.props); + blockedChunk.then(freeze, freeze); + } + return createLazyChunkWrapper(blockedChunk); + } + } else if (__DEV__) { // TODO: We should be freezing the element but currently, we might write into // _debugInfo later. We could move it into _store which remains mutable. - if (initializingChunkBlockedModel !== null) { - const freeze = Object.freeze.bind(Object, element.props); - initializingChunk.then(freeze, freeze); - } else { - Object.freeze(element.props); - } + Object.freeze(element.props); } + return element; } @@ -762,57 +808,129 @@ function getChunk(response: Response, id: number): SomeChunk { return chunk; } -function createModelResolver( - chunk: SomeChunk, +function waitForReference( + referencedChunk: PendingChunk | BlockedChunk, parentObject: Object, key: string, - cyclic: boolean, response: Response, map: (response: Response, model: any) => T, path: Array, -): (value: any) => void { - let blocked; - if (initializingChunkBlockedModel) { - blocked = initializingChunkBlockedModel; - if (!cyclic) { - blocked.deps++; - } +): T { + let handler: InitializationHandler; + if (initializingHandler) { + handler = initializingHandler; + handler.deps++; } else { - blocked = initializingChunkBlockedModel = { - deps: cyclic ? 0 : 1, - value: (null: any), + handler = initializingHandler = { + parent: null, + chunk: null, + value: null, + deps: 1, + errored: false, }; } - return value => { + + function fulfill(value: any): void { for (let i = 1; i < path.length; i++) { + while (value.$$typeof === REACT_LAZY_TYPE) { + // We never expect to see a Lazy node on this path because we encode those as + // separate models. This must mean that we have inserted an extra lazy node + // e.g. to replace a blocked element. We must instead look for it inside. + const chunk: SomeChunk = value._payload; + if (chunk === handler.chunk) { + // This is a reference to the thing we're currently blocking. We can peak + // inside of it to get the value. + value = handler.value; + continue; + } else if (chunk.status === INITIALIZED) { + value = chunk.value; + continue; + } else { + // If we're not yet initialized we need to skip what we've already drilled + // through and then wait for the next value to become available. + path.splice(0, i - 1); + chunk.then(fulfill, reject); + return; + } + } value = value[path[i]]; } parentObject[key] = map(response, value); - // If this is the root object for a model reference, where `blocked.value` + // If this is the root object for a model reference, where `handler.value` // is a stale `null`, the resolved value can be used directly. - if (key === '' && blocked.value === null) { - blocked.value = parentObject[key]; + if (key === '' && handler.value === null) { + handler.value = parentObject[key]; } - blocked.deps--; - if (blocked.deps === 0) { - if (chunk.status !== BLOCKED) { + handler.deps--; + + if (handler.deps === 0) { + const chunk = handler.chunk; + if (chunk === null || chunk.status !== BLOCKED) { return; } const resolveListeners = chunk.value; const initializedChunk: InitializedChunk = (chunk: any); initializedChunk.status = INITIALIZED; - initializedChunk.value = blocked.value; + initializedChunk.value = handler.value; if (resolveListeners !== null) { - wakeChunk(resolveListeners, blocked.value); + wakeChunk(resolveListeners, handler.value); } } - }; -} + } + + function reject(error: mixed): void { + if (handler.errored) { + // We've already errored. We could instead build up an AggregateError + // but if there are multiple errors we just take the first one like + // Promise.all. + return; + } + const blockedValue = handler.value; + handler.errored = true; + handler.value = error; + const chunk = handler.chunk; + if (chunk === null || chunk.status !== BLOCKED) { + return; + } -function createModelReject(chunk: SomeChunk): (error: mixed) => void { - return (error: mixed) => triggerErrorOnChunk(chunk, error); + if (__DEV__) { + if ( + typeof blockedValue === 'object' && + blockedValue !== null && + blockedValue.$$typeof === REACT_ELEMENT_TYPE + ) { + const element = blockedValue; + // Conceptually the error happened inside this Element but right before + // it was rendered. We don't have a client side component to render but + // we can add some DebugInfo to explain that this was conceptually a + // Server side error that errored inside this element. That way any stack + // traces will point to the nearest JSX that errored - e.g. during + // serialization. + const erroredComponent: ReactComponentInfo = { + name: getComponentNameFromType(element.type) || '', + owner: element._owner, + }; + if (enableOwnerStacks) { + // $FlowFixMe[cannot-write] + erroredComponent.stack = element._debugStack; + // $FlowFixMe[cannot-write] + erroredComponent.task = element._debugTask; + } + const chunkDebugInfo: ReactDebugInfo = + chunk._debugInfo || (chunk._debugInfo = []); + chunkDebugInfo.push(erroredComponent); + } + } + + triggerErrorOnChunk(chunk, error); + } + + referencedChunk.then(fulfill, reject); + + // Return a place holder value for now. + return (null: any); } function createServerReferenceProxy, T>( @@ -880,7 +998,7 @@ function getOutlinedModel( if ( typeof chunkValue === 'object' && chunkValue !== null && - (Array.isArray(chunkValue) || + (isArray(chunkValue) || typeof chunkValue[ASYNC_ITERATOR] === 'function' || chunkValue.$$typeof === REACT_ELEMENT_TYPE) && !chunkValue._debugInfo @@ -898,23 +1016,24 @@ function getOutlinedModel( return chunkValue; case PENDING: case BLOCKED: - case CYCLIC: - const parentChunk = initializingChunk; - chunk.then( - createModelResolver( - parentChunk, - parentObject, - key, - chunk.status === CYCLIC, - response, - map, - path, - ), - createModelReject(parentChunk), - ); - return (null: any); + return waitForReference(chunk, parentObject, key, response, map, path); default: - throw chunk.reason; + // This is an error. Instead of erroring directly, we're going to encode this on + // an initialization handler so that we can catch it at the nearest Element. + if (initializingHandler) { + initializingHandler.errored = true; + initializingHandler.value = chunk.reason; + } else { + initializingHandler = { + parent: null, + chunk: null, + value: chunk.reason, + deps: 0, + errored: true, + }; + } + // Placeholder + return (null: any); } } @@ -962,6 +1081,19 @@ function parseModelString( if (value[0] === '$') { if (value === '$') { // A very common symbol. + if (initializingHandler !== null && key === '0') { + // We we already have an initializing handler and we're abound to enter + // a new element, we need to shadow it because we're now in a new scope. + // This is effectively the "begin" or "push" phase of Element parsing. + // We'll pop later when we parse the array itself. + initializingHandler = { + parent: initializingHandler, + chunk: null, + value: null, + deps: 0, + errored: false, + }; + } return REACT_ELEMENT_TYPE; } switch (value[1]) { diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 54d66bdac10d0..5507d4cb6b136 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -1104,6 +1104,46 @@ describe('ReactFlight', () => { }); }); + it('should handle serialization errors in element inside error boundary', async () => { + const ClientErrorBoundary = clientReference(ErrorBoundary); + + const expectedStack = __DEV__ + ? '\n in div' + '\n in ErrorBoundary (at **)' + '\n in App' + : '\n in ErrorBoundary (at **)'; + + function App() { + return ( + +
+ + ); + } + + const transport = ReactNoopFlightServer.render(, { + onError(x) { + if (__DEV__) { + return 'a dev digest'; + } + if (x instanceof Error) { + return `digest("${x.message}")`; + } else if (Array.isArray(x)) { + return `digest([])`; + } else if (typeof x === 'object' && x !== null) { + return `digest({})`; + } + return `digest(${String(x)})`; + }, + }); + + await act(() => { + startTransition(() => { + ReactNoop.render(ReactNoopFlightClient.read(transport)); + }); + }); + }); + it('should include server components in warning stacks', async () => { function Component() { // Trigger key warning diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 8a03e53f30a6f..5c759d9a52cf1 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -4129,7 +4129,7 @@ function beginWork( } case Throw: { // This represents a Component that threw in the reconciliation phase. - // So we'll rethrow here. This might be + // So we'll rethrow here. This might be a Thenable. throw workInProgress.pendingProps; } } diff --git a/packages/react-reconciler/src/getComponentNameFromFiber.js b/packages/react-reconciler/src/getComponentNameFromFiber.js index fbf5f708e2340..cac247f26912b 100644 --- a/packages/react-reconciler/src/getComponentNameFromFiber.js +++ b/packages/react-reconciler/src/getComponentNameFromFiber.js @@ -164,7 +164,7 @@ export default function getComponentNameFromFiber(fiber: Fiber): string | null { break; case Throw: { if (__DEV__) { - // For an error in child position we use the of the inner most parent component. + // For an error in child position we use the name of the inner most parent component. // Whether a Server Component or the parent Fiber. const debugInfo = fiber._debugInfo; if (debugInfo != null) { diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js index 2d50d6fa427e1..90393e435e91f 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -345,6 +345,53 @@ describe('ReactFlightDOMBrowser', () => { expect(container.innerHTML).toBe('
[[1,2,3],[1,2,3]]
'); }); + it('should resolve deduped objects that are themselves blocked', async () => { + let resolveClientComponentChunk; + + const Client = clientExports( + [4, 5], + '42', + '/test.js', + new Promise(resolve => (resolveClientComponentChunk = resolve)), + ); + + const shared = [1, 2, 3, Client]; + + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( +
+ + + {shared /* this will serialize first and block nearest element */} + + + {shared /* this will be referenced inside the blocked element */} +
, + webpackMap, + ), + ); + + function ClientRoot({response}) { + return use(response); + } + + const response = ReactServerDOMClient.createFromReadableStream(stream); + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + await act(() => { + root.render(); + }); + + expect(container.innerHTML).toBe(''); + + await act(() => { + resolveClientComponentChunk(); + }); + + expect(container.innerHTML).toBe('
1234512345
'); + }); + it('should progressively reveal server components', async () => { let reportedErrors = []; diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js index 90597d106b83e..47418276dbb91 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js @@ -224,14 +224,12 @@ describe('ReactFlightDOMEdge', () => { this: {is: 'a large objected'}, with: {many: 'properties in it'}, }; - const props = { - items: new Array(30).fill(obj), - }; + const props = {root:
{new Array(30).fill(obj)}
}; const stream = ReactServerDOMServer.renderToReadableStream(props); const [stream1, stream2] = passThrough(stream).tee(); const serializedContent = await readResult(stream1); - expect(serializedContent.length).toBeLessThan(470); + expect(serializedContent.length).toBeLessThan(1100); const result = await ReactServerDOMClient.createFromReadableStream( stream2, @@ -242,10 +240,13 @@ describe('ReactFlightDOMEdge', () => { }, }, ); + // TODO: Cyclic references currently cause a Lazy wrapper which is not ideal. + const resultElement = result.root._init(result.root._payload); // Should still match the result when parsed - expect(result).toEqual(props); - expect(result.items[5]).toBe(result.items[10]); // two random items are the same instance - // TODO: items[0] is not the same as the others in this case + expect(resultElement).toEqual(props.root); + expect(resultElement.props.children[5]).toBe( + resultElement.props.children[10], + ); // two random items are the same instance }); it('should execute repeated server components only once', async () => { diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 670c11c7c53cf..3bbf3cc9634ed 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1054,13 +1054,14 @@ function renderFunctionComponent( request.pendingChunks++; const componentDebugID = debugID; - componentDebugInfo = { + componentDebugInfo = ({ name: componentName, env: request.environmentName, owner: owner, - }; + }: ReactComponentInfo); if (enableOwnerStacks) { - (componentDebugInfo: any).stack = stack; + // $FlowFixMe[cannot-write] + componentDebugInfo.stack = stack; } // We outline this model eagerly so that we can refer to by reference as an owner. // If we had a smarter way to dedupe we might not have to do this if there ends up @@ -2076,20 +2077,19 @@ function renderModel( task.keyPath = prevKeyPath; task.implicitSlot = prevImplicitSlot; + // Something errored. We'll still send everything we have up until this point. + request.pendingChunks++; + const errorId = request.nextChunkId++; + const digest = logRecoverableError(request, x); + emitErrorChunk(request, errorId, digest, x); if (wasReactNode) { - // 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++; - const digest = logRecoverableError(request, x); - emitErrorChunk(request, errorId, digest, x); return serializeLazyID(errorId); } - // Something errored but it was not in a React Node. There's no need to serialize - // it by value because it'll just error the whole parent row anyway so we can - // just stop any siblings and error the whole parent row. - throw x; + // If we don't know if it was a React Node we render a direct reference and let + // the client deal with it. + return serializeByValueID(errorId); } } @@ -2117,6 +2117,7 @@ function renderModelDestructive( if (typeof value === 'object') { switch ((value: any).$$typeof) { case REACT_ELEMENT_TYPE: { + let elementReference = null; const writtenObjects = request.writtenObjects; if (task.keyPath !== null || task.implicitSlot) { // If we're in some kind of context we can't reuse the result of this render or @@ -2145,10 +2146,8 @@ function renderModelDestructive( if (parentReference !== undefined) { // If the parent has a reference, we can refer to this object indirectly // through the property name inside that parent. - writtenObjects.set( - value, - parentReference + ':' + parentPropertyName, - ); + elementReference = parentReference + ':' + parentPropertyName; + writtenObjects.set(value, elementReference); } } } @@ -2183,7 +2182,7 @@ function renderModelDestructive( } // Attempt to render the Server Component. - return renderElement( + const newChild = renderElement( request, task, element.type, @@ -2199,6 +2198,18 @@ function renderModelDestructive( : null, __DEV__ && enableOwnerStacks ? element._store.validated : 0, ); + if ( + typeof newChild === 'object' && + newChild !== null && + elementReference !== null + ) { + // If this element renders another object, we can now refer to that object through + // the same location as this element. + if (!writtenObjects.has(newChild)) { + writtenObjects.set(newChild, elementReference); + } + } + return newChild; } case REACT_LAZY_TYPE: { // Reset the task's thenable state before continuing. If there was one, it was @@ -2478,15 +2489,16 @@ function renderModelDestructive( ) { // This looks like a ReactComponentInfo. We can't serialize the ConsoleTask object so we // need to omit it before serializing. - const componentDebugInfo = { + const componentDebugInfo: Omit = { name: value.name, env: value.env, - owner: value.owner, + owner: (value: any).owner, }; if (enableOwnerStacks) { - (componentDebugInfo: any).stack = (value: any).stack; + // $FlowFixMe[cannot-write] + componentDebugInfo.stack = (value: any).stack; } - return (componentDebugInfo: any); + return componentDebugInfo; } if (objectName(value) !== 'Object') {