From dd4950c90e65a6c53870fd61d2aea67bce7d6713 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Tue, 31 May 2022 14:53:32 -0700 Subject: [PATCH] [Flight] Implement useId hook (#24172) * Implements useId hook for Flight server. The approach for ids for Flight is different from Fizz/Client where there is a need for determinancy. Flight rendered elements will not be rendered on the client and as such the ids generated in a request only need to be unique. However since FLight does support refetching subtrees it is possible a client will need to patch up a part of the tree rather than replacing the entire thing so it is not safe to use a simple incrementing counter. To solve for this we allow the caller to specify a prefix. On an initial fetch it is likely this will be empty but on refetches or subtrees we expect to have a client `useId` provide the prefix since it will guaranteed be unique for that subtree and thus for the entire tree. It is also possible that we will automatically provide prefixes based on a client/Fizz useId on refetches in addition to the core change I also modified the structure of options for renderToReadableStream where `onError`, `context`, and the new `identifierPrefix` are properties of an Options object argument to avoid the clumsiness of a growing list of optional function arguments. * defend against useId call outside of rendering * switch to S from F for Server Component ids * default to empty string identifier prefix * Add a test demonstrating that there is no warning when double rendering on the client a server component that used useId * lints and gates --- .../src/__tests__/ReactFlight-test.js | 100 +++++++++++++++++- .../src/ReactNoopFlightServer.js | 11 +- .../src/ReactFlightDOMRelayServer.js | 3 + .../src/ReactFlightDOMServerBrowser.js | 6 +- .../src/ReactFlightDOMServerNode.js | 5 +- packages/react-server/src/ReactFlightHooks.js | 22 +++- .../react-server/src/ReactFlightServer.js | 19 +++- scripts/error-codes/codes.json | 3 +- 8 files changed, 149 insertions(+), 20 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 6213f0b72e08..505b9072ff7c 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -512,6 +512,99 @@ describe('ReactFlight', () => { ); }); + describe('Hooks', () => { + function DivWithId({children}) { + const id = React.useId(); + return
{children}
; + } + + it('should support useId', () => { + function App() { + return ( + <> + + + + ); + } + + const transport = ReactNoopFlightServer.render(); + act(() => { + ReactNoop.render(ReactNoopFlightClient.read(transport)); + }); + expect(ReactNoop).toMatchRenderedOutput( + <> +
+
+ , + ); + }); + + it('accepts an identifier prefix that prefixes generated ids', () => { + function App() { + return ( + <> + + + + ); + } + + const transport = ReactNoopFlightServer.render(, { + identifierPrefix: 'foo', + }); + act(() => { + ReactNoop.render(ReactNoopFlightClient.read(transport)); + }); + expect(ReactNoop).toMatchRenderedOutput( + <> +
+
+ , + ); + }); + + it('[TODO] it does not warn if you render a server element passed to a client module reference twice on the client when using useId', async () => { + // @TODO Today if you render a server component with useId and pass it to a client component and that client component renders the element in two or more + // places the id used on the server will be duplicated in the client. This is a deviation from the guarantees useId makes for Fizz/Client and is a consequence + // of the fact that the server component is actually rendered on the server and is reduced to a set of host elements before being passed to the Client component + // so the output passed to the Client has no knowledge of the useId use. In the future we would like to add a DEV warning when this happens. For now + // we just accept that it is a nuance of useId in Flight + function App() { + const id = React.useId(); + const div =
{id}
; + return ; + } + + function ClientDoubler({el}) { + Scheduler.unstable_yieldValue('ClientDoubler'); + return ( + <> + {el} + {el} + + ); + } + + const ClientDoublerModuleRef = moduleReference(ClientDoubler); + + const transport = ReactNoopFlightServer.render(); + expect(Scheduler).toHaveYielded([]); + + act(() => { + ReactNoop.render(ReactNoopFlightClient.read(transport)); + }); + + expect(Scheduler).toHaveYielded(['ClientDoubler']); + expect(ReactNoop).toMatchRenderedOutput( + <> +
:S1:
+
:S1:
+ , + ); + }); + }); + describe('ServerContext', () => { // @gate enableServerContext it('supports basic createServerContext usage', () => { @@ -759,15 +852,14 @@ describe('ReactFlight', () => { function Bar() { return {React.useContext(ServerContext)}; } - const transport = ReactNoopFlightServer.render(, {}, [ - ['ServerContext', 'Override'], - ]); + const transport = ReactNoopFlightServer.render(, { + context: [['ServerContext', 'Override']], + }); act(() => { const flightModel = ReactNoopFlightClient.read(transport); ReactNoop.render(flightModel); }); - expect(ReactNoop).toMatchRenderedOutput(Override); }); diff --git a/packages/react-noop-renderer/src/ReactNoopFlightServer.js b/packages/react-noop-renderer/src/ReactNoopFlightServer.js index 1c607befe74b..3a84a06d0843 100644 --- a/packages/react-noop-renderer/src/ReactNoopFlightServer.js +++ b/packages/react-noop-renderer/src/ReactNoopFlightServer.js @@ -61,20 +61,19 @@ const ReactNoopFlightServer = ReactFlightServer({ type Options = { onError?: (error: mixed) => void, + context?: Array<[string, ServerContextJSONValue]>, + identifierPrefix?: string, }; -function render( - model: ReactModel, - options?: Options, - context?: Array<[string, ServerContextJSONValue]>, -): Destination { +function render(model: ReactModel, options?: Options): Destination { const destination: Destination = []; const bundlerConfig = undefined; const request = ReactNoopFlightServer.createRequest( model, bundlerConfig, options ? options.onError : undefined, - context, + options ? options.context : undefined, + options ? options.identifierPrefix : undefined, ); ReactNoopFlightServer.startWork(request); ReactNoopFlightServer.startFlowing(request, destination); diff --git a/packages/react-server-dom-relay/src/ReactFlightDOMRelayServer.js b/packages/react-server-dom-relay/src/ReactFlightDOMRelayServer.js index 7feb6e3f0eff..5e65921193ba 100644 --- a/packages/react-server-dom-relay/src/ReactFlightDOMRelayServer.js +++ b/packages/react-server-dom-relay/src/ReactFlightDOMRelayServer.js @@ -21,6 +21,7 @@ import { type Options = { onError?: (error: mixed) => void, + identifierPrefix?: string, }; function render( @@ -33,6 +34,8 @@ function render( model, config, options ? options.onError : undefined, + undefined, // not currently set up to supply context overrides + options ? options.identifierPrefix : undefined, ); startWork(request); startFlowing(request, destination); diff --git a/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js b/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js index a00eb8c44106..bfd47cc40a69 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js +++ b/packages/react-server-dom-webpack/src/ReactFlightDOMServerBrowser.js @@ -19,19 +19,21 @@ import { type Options = { onError?: (error: mixed) => void, + context?: Array<[string, ServerContextJSONValue]>, + identifierPrefix?: string, }; function renderToReadableStream( model: ReactModel, webpackMap: BundlerConfig, options?: Options, - context?: Array<[string, ServerContextJSONValue]>, ): ReadableStream { const request = createRequest( model, webpackMap, options ? options.onError : undefined, - context, + options ? options.context : undefined, + options ? options.identifierPrefix : undefined, ); const stream = new ReadableStream( { diff --git a/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js b/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js index 76952b6d5425..b89a91a456b8 100644 --- a/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js +++ b/packages/react-server-dom-webpack/src/ReactFlightDOMServerNode.js @@ -24,6 +24,8 @@ function createDrainHandler(destination, request) { type Options = { onError?: (error: mixed) => void, + context?: Array<[string, ServerContextJSONValue]>, + identifierPrefix?: string, }; type PipeableStream = {| @@ -40,7 +42,8 @@ function renderToPipeableStream( model, webpackMap, options ? options.onError : undefined, - context, + options ? options.context : undefined, + options ? options.identifierPrefix : undefined, ); let hasStartedFlowing = false; startWork(request); diff --git a/packages/react-server/src/ReactFlightHooks.js b/packages/react-server/src/ReactFlightHooks.js index 88a2eac86ca2..5b10d8c7a1f6 100644 --- a/packages/react-server/src/ReactFlightHooks.js +++ b/packages/react-server/src/ReactFlightHooks.js @@ -8,10 +8,21 @@ */ import type {Dispatcher as DispatcherType} from 'react-reconciler/src/ReactInternalTypes'; +import type {Request} from './ReactFlightServer'; import type {ReactServerContext} from 'shared/ReactTypes'; import {REACT_SERVER_CONTEXT_TYPE} from 'shared/ReactSymbols'; import {readContext as readContextImpl} from './ReactFlightNewContext'; +let currentRequest = null; + +export function prepareToUseHooksForRequest(request: Request) { + currentRequest = request; +} + +export function resetHooksForRequest() { + currentRequest = null; +} + function readContext(context: ReactServerContext): T { if (__DEV__) { if (context.$$typeof !== REACT_SERVER_CONTEXT_TYPE) { @@ -61,7 +72,7 @@ export const Dispatcher: DispatcherType = { useLayoutEffect: (unsupportedHook: any), useImperativeHandle: (unsupportedHook: any), useEffect: (unsupportedHook: any), - useId: (unsupportedHook: any), + useId, useMutableSource: (unsupportedHook: any), useSyncExternalStore: (unsupportedHook: any), useCacheRefresh(): (?() => T, ?T) => void { @@ -91,3 +102,12 @@ export function setCurrentCache(cache: Map | null) { export function getCurrentCache() { return currentCache; } + +function useId(): string { + if (currentRequest === null) { + throw new Error('useId can only be used while React is rendering'); + } + const id = currentRequest.identifierCount++; + // use 'S' for Flight components to distinguish from 'R' and 'r' in Fizz/Client + return ':' + currentRequest.identifierPrefix + 'S' + id.toString(32) + ':'; +} diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index e08f308a32fd..d1399ce63ae8 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -39,7 +39,13 @@ import { isModuleReference, } from './ReactFlightServerConfig'; -import {Dispatcher, getCurrentCache, setCurrentCache} from './ReactFlightHooks'; +import { + Dispatcher, + getCurrentCache, + prepareToUseHooksForRequest, + resetHooksForRequest, + setCurrentCache, +} from './ReactFlightHooks'; import { pushProvider, popProvider, @@ -102,14 +108,12 @@ export type Request = { writtenSymbols: Map, writtenModules: Map, writtenProviders: Map, + identifierPrefix: string, + identifierCount: number, onError: (error: mixed) => void, toJSON: (key: string, value: ReactModel) => ReactJSONValue, }; -export type Options = { - onError?: (error: mixed) => void, -}; - const ReactCurrentDispatcher = ReactSharedInternals.ReactCurrentDispatcher; function defaultErrorHandler(error: mixed) { @@ -126,6 +130,7 @@ export function createRequest( bundlerConfig: BundlerConfig, onError: void | ((error: mixed) => void), context?: Array<[string, ServerContextJSONValue]>, + identifierPrefix?: string, ): Request { const pingedSegments = []; const request = { @@ -143,6 +148,8 @@ export function createRequest( writtenSymbols: new Map(), writtenModules: new Map(), writtenProviders: new Map(), + identifierPrefix: identifierPrefix || '', + identifierCount: 1, onError: onError === undefined ? defaultErrorHandler : onError, toJSON: function(key: string, value: ReactModel): ReactJSONValue { return resolveModelToJSON(request, this, key, value); @@ -826,6 +833,7 @@ function performWork(request: Request): void { const prevCache = getCurrentCache(); ReactCurrentDispatcher.current = Dispatcher; setCurrentCache(request.cache); + prepareToUseHooksForRequest(request); try { const pingedSegments = request.pingedSegments; @@ -843,6 +851,7 @@ function performWork(request: Request): void { } finally { ReactCurrentDispatcher.current = prevDispatcher; setCurrentCache(prevCache); + resetHooksForRequest(); } } diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 7a93b9a0ac64..a8c8810bbe49 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -417,5 +417,6 @@ "429": "ServerContext: %s already defined", "430": "ServerContext can only have a value prop and children. Found: %s", "431": "React elements are not allowed in ServerContext", - "432": "This Suspense boundary was aborted by the server" + "432": "This Suspense boundary was aborted by the server", + "433": "useId can only be used while React is rendering" }