@samitouri / QOS-React / commits / dff50825c6

[Flight] Track owner/stack where the Flight Client reads as the root (#30933)

This means that the owner of a Component rendered on the remote server becomes the Component on this server. Ideally we'd support this for the Client side too. In particular Fiber but currently ReactComponentInfo's owner is typed as only supporting other ReactComponentInfo and it's a bigger lift to support that.

Sebastian Markbåge committed Sep 12, 2024 at 17:19 UTC dff50825c6ca4c04c79fd7fe2d2d345ea5e29f87
3 files changed +123 -12
packages/react-client/src/ReactFlightClient.js
+55 -11
@@ -86,14 +86,19 @@ import isArray from 'shared/isArray';
86
87 import * as React from 'react';
88
89 +import type {SharedStateServer} from 'react/src/ReactSharedInternalsServer';
90 +import type {SharedStateClient} from 'react/src/ReactSharedInternalsClient';
91 +
92 // TODO: This is an unfortunate hack. We shouldn't feature detect the internals
93 // like this. It's just that for now we support the same build of the Flight
94 // client both in the RSC environment, in the SSR environments as well as the
95 // browser client. We should probably have a separate RSC build. This is DEV
96 // only though.
94 -const ReactSharedInternals =
97 +const ReactSharedInteralsServer: void | SharedStateServer = (React: any)
98 + .__SERVER_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE;
99 +const ReactSharedInternals: SharedStateServer | SharedStateClient =
100 React.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE ||
96 - React.__SERVER_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE;
101 + ReactSharedInteralsServer;
102
103 export type {CallServerCallback, EncodeFormActionCallback};
104
@@ -277,6 +282,8 @@ export type Response = {
282 _rowLength: number, // remaining bytes in the row. 0 indicates that we're looking for a newline.
283 _buffer: Array<Uint8Array>, // chunks received so far as part of this row
284 _tempRefs: void | TemporaryReferenceSet, // the set temporary references can be resolved from
285 + _debugRootOwner?: null | ReactComponentInfo, // DEV-only
286 + _debugRootStack?: null | Error, // DEV-only
287 _debugRootTask?: null | ConsoleTask, // DEV-only
288 _debugFindSourceMapURL?: void | FindSourceMapURLCallback, // DEV-only
289 _replayConsole: boolean, // DEV-only
@@ -672,7 +679,7 @@ function createElement(
679 type,
680 key,
681 props,
675 - _owner: owner,
682 + _owner: __DEV__ && owner === null ? response._debugRootOwner : owner,
683 }: any);
684 Object.defineProperty(element, 'ref', {
685 enumerable: false,
@@ -699,7 +706,7 @@ function createElement(
706 props,
707
708 // Record the component responsible for creating this element.
702 - _owner: owner,
709 + _owner: __DEV__ && owner === null ? response._debugRootOwner : owner,
710 }: any);
711 }
712
@@ -733,7 +740,11 @@ function createElement(
740 env = owner.env;
741 }
742 let normalizedStackTrace: null | Error = null;
736 - if (stack !== null) {
743 + if (owner === null && response._debugRootStack != null) {
744 + // We override the stack if we override the owner since the stack where the root JSX
745 + // was created on the server isn't very useful but where the request was made is.
746 + normalizedStackTrace = response._debugRootStack;
747 + } else if (stack !== null) {
748 // We create a fake stack and then create an Error object inside of it.
749 // This means that the stack trace is now normalized into the native format
750 // of the browser and the stack frames will have been registered with
@@ -821,8 +832,10 @@ function createElement(
832 if (enableOwnerStacks) {
833 // $FlowFixMe[cannot-write]
834 erroredComponent.debugStack = element._debugStack;
824 - // $FlowFixMe[cannot-write]
825 - erroredComponent.debugTask = element._debugTask;
835 + if (supportsCreateTask) {
836 + // $FlowFixMe[cannot-write]
837 + erroredComponent.debugTask = element._debugTask;
838 + }
839 }
840 erroredChunk._debugInfo = [erroredComponent];
841 }
@@ -998,8 +1011,10 @@ function waitForReference<T>(
1011 if (enableOwnerStacks) {
1012 // $FlowFixMe[cannot-write]
1013 erroredComponent.debugStack = element._debugStack;
1001 - // $FlowFixMe[cannot-write]
1002 - erroredComponent.debugTask = element._debugTask;
1014 + if (supportsCreateTask) {
1015 + // $FlowFixMe[cannot-write]
1016 + erroredComponent.debugTask = element._debugTask;
1017 + }
1018 }
1019 const chunkDebugInfo: ReactDebugInfo =
1020 chunk._debugInfo || (chunk._debugInfo = []);
@@ -1408,6 +1423,25 @@ function ResponseInstance(
1423 this._buffer = [];
1424 this._tempRefs = temporaryReferences;
1425 if (__DEV__) {
1426 + // TODO: The Flight Client can be used in a Client Environment too and we should really support
1427 + // getting the owner there as well, but currently the owner of ReactComponentInfo is typed as only
1428 + // supporting other ReactComponentInfo as owners (and not Fiber or Fizz's ComponentStackNode).
1429 + // We need to update all the callsites consuming ReactComponentInfo owners to support those.
1430 + // In the meantime we only check ReactSharedInteralsServer since we know that in an RSC environment
1431 + // the only owners will be ReactComponentInfo.
1432 + const rootOwner: null | ReactComponentInfo =
1433 + ReactSharedInteralsServer === undefined ||
1434 + ReactSharedInteralsServer.A === null
1435 + ? null
1436 + : (ReactSharedInteralsServer.A.getOwner(): any);
1437 +
1438 + this._debugRootOwner = rootOwner;
1439 + this._debugRootStack =
1440 + rootOwner !== null
1441 + ? // TODO: Consider passing the top frame in so we can avoid internals showing up.
1442 + new Error('react-stack-top-frame')
1443 + : null;
1444 +
1445 const rootEnv = environmentName === undefined ? 'Server' : environmentName;
1446 if (supportsCreateTask) {
1447 // Any stacks that appear on the server need to be rooted somehow on the client
@@ -2308,7 +2342,16 @@ function resolveDebugInfo(
2342 const env =
2343 debugInfo.env === undefined ? response._rootEnvironmentName : debugInfo.env;
2344 initializeFakeTask(response, debugInfo, env);
2311 - initializeFakeStack(response, debugInfo);
2345 + if (debugInfo.owner === null && response._debugRootOwner != null) {
2346 + // $FlowFixMe
2347 + debugInfo.owner = response._debugRootOwner;
2348 + // We override the stack if we override the owner since the stack where the root JSX
2349 + // was created on the server isn't very useful but where the request was made is.
2350 + // $FlowFixMe
2351 + debugInfo.debugStack = response._debugRootStack;
2352 + } else {
2353 + initializeFakeStack(response, debugInfo);
2354 + }
2355
2356 const chunk = getChunk(response, id);
2357 const chunkDebugInfo: ReactDebugInfo =
@@ -2344,7 +2387,8 @@ const replayConsoleWithCallStack = {
2387 // There really shouldn't be anything else on the stack atm.
2388 const prevStack = ReactSharedInternals.getCurrentStack;
2389 ReactSharedInternals.getCurrentStack = getCurrentStackInDEV;
2347 - currentOwnerInDEV = owner;
2390 + currentOwnerInDEV =
2391 + owner === null ? (response._debugRootOwner: any) : owner;
2392
2393 try {
2394 const callStack = buildFakeCallStack(
packages/react-client/src/__tests__/ReactFlight-test.js
+67
@@ -24,6 +24,10 @@ function normalizeCodeLocInfo(str) {
24 return (
25 str &&
26 str.replace(/^ +(?:at|in) ([\S]+)[^\n]*/gm, function (m, name) {
27 + const dot = name.lastIndexOf('.');
28 + if (dot !== -1) {
29 + name = name.slice(dot + 1);
30 + }
31 return ' in ' + name + (/\d/.test(m) ? ' (at **)' : '');
32 })
33 );
@@ -3124,6 +3128,69 @@ describe('ReactFlight', () => {
3128 );
3129 });
3130
3131 + // @gate __DEV__ && enableOwnerStacks
3132 + it('can track owner for a flight response created in another render', async () => {
3133 + jest.resetModules();
3134 + jest.mock('react', () => ReactServer);
3135 + // For this to work the Flight Client needs to be the react-server version.
3136 + const ReactNoopFlightClienOnTheServer = require('react-noop-renderer/flight-client');
3137 + jest.resetModules();
3138 + jest.mock('react', () => React);
3139 +
3140 + let stack;
3141 +
3142 + function Component() {
3143 + stack = ReactServer.captureOwnerStack();
3144 + return ReactServer.createElement('span', null, 'hi');
3145 + }
3146 +
3147 + const ClientComponent = clientReference(Component);
3148 +
3149 + function ThirdPartyComponent() {
3150 + return ReactServer.createElement(ClientComponent);
3151 + }
3152 +
3153 + // This is rendered outside the render to ensure we don't inherit anything accidental
3154 + // by being in the same environment which would make it seem like it works when it doesn't.
3155 + const thirdPartyTransport = ReactNoopFlightServer.render(
3156 + {children: ReactServer.createElement(ThirdPartyComponent)},
3157 + {
3158 + environmentName: 'third-party',
3159 + },
3160 + );
3161 +
3162 + async function fetchThirdParty() {
3163 + return ReactNoopFlightClienOnTheServer.read(thirdPartyTransport);
3164 + }
3165 +
3166 + async function FirstPartyComponent() {
3167 + // This component fetches from a third party
3168 + const thirdParty = await fetchThirdParty();
3169 + return thirdParty.children;
3170 + }
3171 + function App() {
3172 + return ReactServer.createElement(FirstPartyComponent);
3173 + }
3174 +
3175 + const transport = ReactNoopFlightServer.render(
3176 + ReactServer.createElement(App),
3177 + );
3178 +
3179 + await act(async () => {
3180 + const root = await ReactNoopFlightClient.read(transport);
3181 + ReactNoop.render(root);
3182 + });
3183 +
3184 + expect(normalizeCodeLocInfo(stack)).toBe(
3185 + '\n in ThirdPartyComponent (at **)' +
3186 + '\n in createResponse (at **)' + // These two internal frames should
3187 + '\n in read (at **)' + // ideally not be included.
3188 + '\n in fetchThirdParty (at **)' +
3189 + '\n in FirstPartyComponent (at **)' +
3190 + '\n in App (at **)',
3191 + );
3192 + });
3193 +
3194 // @gate __DEV__ && enableOwnerStacks
3195 it('can get the component owner stacks for onError in dev', async () => {
3196 const thrownError = new Error('hi');
packages/react-server/src/ReactFlightServer.js
+1 -1
@@ -2271,7 +2271,7 @@ function isReactComponentInfo(value: any): boolean {
2271 typeof value.debugTask.run === 'function') ||
2272 value.debugStack instanceof Error) &&
2273 (enableOwnerStacks
2274 - ? isArray((value: any).stack)
2274 + ? isArray((value: any).stack) || (value: any).stack === null
2275 : typeof (value: any).stack === 'undefined') &&
2276 typeof value.name === 'string' &&
2277 typeof value.env === 'string' &&