From cb43e441713d3e290e2e14e89bc8f9c69a503bee Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 11 Jun 2024 21:36:56 -0400 Subject: [PATCH 1/3] Let environmentName vary over time by making it a function of string This lets the environment name vary within a request by the context it's being executed in. We could potentially also pass some information to it for context about what is being asked for. --- .../src/ReactNoopFlightServer.js | 2 +- .../src/ReactFlightDOMServerNode.js | 2 +- .../src/ReactFlightDOMServerBrowser.js | 2 +- .../src/ReactFlightDOMServerEdge.js | 2 +- .../src/ReactFlightDOMServerNode.js | 2 +- .../src/ReactFlightDOMServerBrowser.js | 2 +- .../src/ReactFlightDOMServerEdge.js | 2 +- .../src/ReactFlightDOMServerNode.js | 2 +- packages/react-server/src/ReactFlightServer.js | 16 ++++++++++------ 9 files changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/react-noop-renderer/src/ReactNoopFlightServer.js b/packages/react-noop-renderer/src/ReactNoopFlightServer.js index cf6f24404c3ed..ef3c738325d41 100644 --- a/packages/react-noop-renderer/src/ReactNoopFlightServer.js +++ b/packages/react-noop-renderer/src/ReactNoopFlightServer.js @@ -67,7 +67,7 @@ const ReactNoopFlightServer = ReactFlightServer({ }); type Options = { - environmentName?: string, + environmentName?: string | (() => string), identifierPrefix?: string, onError?: (error: mixed) => void, onPostpone?: (reason: string) => void, diff --git a/packages/react-server-dom-esm/src/ReactFlightDOMServerNode.js b/packages/react-server-dom-esm/src/ReactFlightDOMServerNode.js index 4219013cc3de1..16891a14641b2 100644 --- a/packages/react-server-dom-esm/src/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-esm/src/ReactFlightDOMServerNode.js @@ -66,7 +66,7 @@ function createCancelHandler(request: Request, reason: string) { } type Options = { - environmentName?: string, + environmentName?: string | (() => string), onError?: (error: mixed) => void, onPostpone?: (reason: string) => void, identifierPrefix?: string, diff --git a/packages/react-server-dom-turbopack/src/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-turbopack/src/ReactFlightDOMServerBrowser.js index e15ed19c074e0..f12c811a40f83 100644 --- a/packages/react-server-dom-turbopack/src/ReactFlightDOMServerBrowser.js +++ b/packages/react-server-dom-turbopack/src/ReactFlightDOMServerBrowser.js @@ -44,7 +44,7 @@ export {createTemporaryReferenceSet} from 'react-server/src/ReactFlightServerTem export type {TemporaryReferenceSet}; type Options = { - environmentName?: string, + environmentName?: string | (() => string), identifierPrefix?: string, signal?: AbortSignal, temporaryReferences?: TemporaryReferenceSet, diff --git a/packages/react-server-dom-turbopack/src/ReactFlightDOMServerEdge.js b/packages/react-server-dom-turbopack/src/ReactFlightDOMServerEdge.js index e15ed19c074e0..f12c811a40f83 100644 --- a/packages/react-server-dom-turbopack/src/ReactFlightDOMServerEdge.js +++ b/packages/react-server-dom-turbopack/src/ReactFlightDOMServerEdge.js @@ -44,7 +44,7 @@ export {createTemporaryReferenceSet} from 'react-server/src/ReactFlightServerTem export type {TemporaryReferenceSet}; type Options = { - environmentName?: string, + environmentName?: string | (() => string), identifierPrefix?: string, signal?: AbortSignal, temporaryReferences?: TemporaryReferenceSet, diff --git a/packages/react-server-dom-turbopack/src/ReactFlightDOMServerNode.js b/packages/react-server-dom-turbopack/src/ReactFlightDOMServerNode.js index f47ce43dc01a2..f8623ee0b51dd 100644 --- a/packages/react-server-dom-turbopack/src/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-turbopack/src/ReactFlightDOMServerNode.js @@ -67,7 +67,7 @@ function createCancelHandler(request: Request, reason: string) { } type Options = { - environmentName?: string, + environmentName?: string | (() => string), onError?: (error: mixed) => void, onPostpone?: (reason: string) => void, identifierPrefix?: string, diff --git a/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js index 0a737903c2918..e446778a71f64 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js +++ b/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js @@ -44,7 +44,7 @@ export {createTemporaryReferenceSet} from 'react-server/src/ReactFlightServerTem export type {TemporaryReferenceSet}; type Options = { - environmentName?: string, + environmentName?: string | (() => string), identifierPrefix?: string, signal?: AbortSignal, temporaryReferences?: TemporaryReferenceSet, diff --git a/packages/react-server-dom-webpack/src/ReactFlightDOMServerEdge.js b/packages/react-server-dom-webpack/src/ReactFlightDOMServerEdge.js index 0a737903c2918..e446778a71f64 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightDOMServerEdge.js +++ b/packages/react-server-dom-webpack/src/ReactFlightDOMServerEdge.js @@ -44,7 +44,7 @@ export {createTemporaryReferenceSet} from 'react-server/src/ReactFlightServerTem export type {TemporaryReferenceSet}; type Options = { - environmentName?: string, + environmentName?: string | (() => string), identifierPrefix?: string, signal?: AbortSignal, temporaryReferences?: TemporaryReferenceSet, diff --git a/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js b/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js index ae71dde45c95f..5560b1c033470 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js @@ -67,7 +67,7 @@ function createCancelHandler(request: Request, reason: string) { } type Options = { - environmentName?: string, + environmentName?: string | (() => string), onError?: (error: mixed) => void, onPostpone?: (reason: string) => void, identifierPrefix?: string, diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 3bbf3cc9634ed..89c125c3a49cd 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -425,7 +425,7 @@ export type Request = { onError: (error: mixed) => ?string, onPostpone: (reason: string) => void, // DEV-only - environmentName: string, + environmentName: () => string, didWarnForKey: null | WeakSet, }; @@ -481,7 +481,7 @@ function RequestInstance( onError: void | ((error: mixed) => ?string), identifierPrefix?: string, onPostpone: void | ((reason: string) => void), - environmentName: void | string, + environmentName: void | string | (() => string), temporaryReferences: void | TemporaryReferenceSet, ) { if ( @@ -531,7 +531,11 @@ function RequestInstance( if (__DEV__) { this.environmentName = - environmentName === undefined ? 'Server' : environmentName; + environmentName === undefined + ? () => 'Server' + : typeof environmentName !== 'function' + ? () => environmentName + : environmentName; this.didWarnForKey = null; } const rootTask = createTask(this, model, null, false, abortSet); @@ -544,7 +548,7 @@ export function createRequest( onError: void | ((error: mixed) => ?string), identifierPrefix?: string, onPostpone: void | ((reason: string) => void), - environmentName: void | string, + environmentName: void | string | (() => string), temporaryReferences: void | TemporaryReferenceSet, ): Request { // $FlowFixMe[invalid-constructor]: the shapes are exact here but Flow doesn't like constructors @@ -1056,7 +1060,7 @@ function renderFunctionComponent( const componentDebugID = debugID; componentDebugInfo = ({ name: componentName, - env: request.environmentName, + env: request.environmentName(), owner: owner, }: ReactComponentInfo); if (enableOwnerStacks) { @@ -3252,7 +3256,7 @@ function emitConsoleChunk( } // TODO: Don't double badge if this log came from another Flight Client. - const env = request.environmentName; + const env = request.environmentName(); const payload = [methodName, stackTrace, owner, env]; // $FlowFixMe[method-unbinding] payload.push.apply(payload, args); From e4694c5cc5609f3cb8ece86a7d951a5ced3b8781 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 11 Jun 2024 21:48:16 -0400 Subject: [PATCH 2/3] Keep track of the latest environment per task and emit changes That way if the environment changes inside a component we track what it was before entering and what it was after leaving. --- .../react-server/src/ReactFlightServer.js | 36 ++++++++++++++++--- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 89c125c3a49cd..405401a8b3421 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -393,6 +393,7 @@ type Task = { keyPath: null | string, // parent server component keys implicitSlot: boolean, // true if the root server component of this sequence had a null key thenableState: ThenableState | null, + environmentName: string, // DEV-only. Used to track if the environment for this task changed. }; interface Reference {} @@ -1053,14 +1054,14 @@ function renderFunctionComponent( componentDebugInfo = (prevThenableState: any)._componentDebugInfo; } else { // This is a new component in the same task so we can emit more debug info. + const componentDebugID = debugID; const componentName = (Component: any).displayName || Component.name || ''; + const componentEnv = request.environmentName(); request.pendingChunks++; - - const componentDebugID = debugID; componentDebugInfo = ({ name: componentName, - env: request.environmentName(), + env: componentEnv, owner: owner, }: ReactComponentInfo); if (enableOwnerStacks) { @@ -1073,6 +1074,9 @@ function renderFunctionComponent( outlineModel(request, componentDebugInfo); emitDebugChunk(request, componentDebugID, componentDebugInfo); + // We've emitted the latest environment for this task so we track that. + task.environmentName = componentEnv; + if (enableOwnerStacks) { warnForMissingKey(request, key, validated, componentDebugInfo); } @@ -1648,7 +1652,7 @@ function createTask( request.writtenObjects.set(model, serializeByValueID(id)); } } - const task: Task = { + const task: Task = (({ id, status: PENDING, model, @@ -1701,7 +1705,10 @@ function createTask( return renderModel(request, task, parent, parentPropertyName, value); }, thenableState: null, - }; + }: Omit): any); + if (__DEV__) { + task.environmentName = request.environmentName(); + } abortSet.add(task); return task; } @@ -3424,6 +3431,15 @@ function retryTask(request: Request, task: Task): void { // any future references. request.writtenObjects.set(resolvedModel, serializeByValueID(task.id)); + if (__DEV__) { + const currentEnv = request.environmentName(); + if (currentEnv !== task.environmentName) { + // The environment changed since we last emitted any debug information for this + // task. We emit an entry that just includes the environment name change. + emitDebugChunk(request, task.id, {env: currentEnv}); + } + } + // Object might contain unresolved values like additional elements. // This is simulating what the JSON loop would do if this was part of it. emitChunk(request, task, resolvedModel); @@ -3432,6 +3448,16 @@ function retryTask(request: Request, task: Task): void { // We don't need to escape it again so it's not passed the toJSON replacer. // $FlowFixMe[incompatible-type] stringify can return null for undefined but we never do const json: string = stringify(resolvedModel); + + if (__DEV__) { + const currentEnv = request.environmentName(); + if (currentEnv !== task.environmentName) { + // The environment changed since we last emitted any debug information for this + // task. We emit an entry that just includes the environment name change. + emitDebugChunk(request, task.id, {env: currentEnv}); + } + } + emitModelChunk(request, task.id, json); } From a9a89d0282189f8da8b1b07388a0b67bcaa1dab5 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 11 Jun 2024 21:53:30 -0400 Subject: [PATCH 3/3] Test --- .../src/__tests__/ReactFlight-test.js | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 5507d4cb6b136..bd2d98736addd 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -2565,6 +2565,50 @@ describe('ReactFlight', () => { ); }); + it('can change the environment name inside a component', async () => { + let env = 'A'; + function Component(props) { + env = 'B'; + return
hi
; + } + + const transport = ReactNoopFlightServer.render( + { + greeting: , + }, + { + environmentName() { + return env; + }, + }, + ); + + await act(async () => { + const rootModel = await ReactNoopFlightClient.read(transport); + const greeting = rootModel.greeting; + expect(getDebugInfo(greeting)).toEqual( + __DEV__ + ? [ + { + name: 'Component', + env: 'A', + owner: null, + stack: gate(flag => flag.enableOwnerStacks) + ? ' in Object. (at **)' + : undefined, + }, + { + env: 'B', + }, + ] + : undefined, + ); + ReactNoop.render(greeting); + }); + + expect(ReactNoop).toMatchRenderedOutput(
hi
); + }); + // @gate enableServerComponentLogs && __DEV__ it('replays logs, but not onError logs', async () => { function foo() {