@samitouri / QOS-React / commits / 19bd26beb6

[Flight/DevTools] Pass the Server Component's "key" as Part of the ReactComponentInfo (#30703)

Supports showing the key in DevTools on the Server Component that the key was applied to. We can also use this to reconcile to preserve instance equality when they're reordered. One thing that's a bit weird about this is that if you provide an explicit key on a Server Component that alone doesn't have any semantics. It's because we pass the key down and let the nearest child inherit the key or get prefixed by the key. So you might see the same key as a prefix on the child of the Server Component too which might be a bit confusing. We could remove the prefix from children but that might also be a bit confusing if they collide. The div in this case doesn't have a key explicitly specified. It gets it from the Server Component parent. <img width="1107" alt="Screenshot 2024-08-14 at 10 06 36 PM" src="https://github.com/user-attachments/assets/cfc517cc-e737-44c3-a1be-050049267ee2"> Overall keys get a bit confusing when you apply filter. Especially since it's so common to actually apply the key on a Host Instance. So you often don't see the key.

Sebastian Markbåge committed Aug 15, 2024 at 11:04 UTC 19bd26beb689e554fceb0b929dc5199be8cba594
6 files changed +64 -27
packages/react-client/src/__tests__/ReactFlight-test.js
+12
@@ -299,6 +299,7 @@ describe('ReactFlight', () => {
299 {
300 name: 'Greeting',
301 env: 'Server',
302 + key: null,
303 owner: null,
304 stack: gate(flag => flag.enableOwnerStacks)
305 ? ' in Object.<anonymous> (at **)'
@@ -337,6 +338,7 @@ describe('ReactFlight', () => {
338 {
339 name: 'Greeting',
340 env: 'Server',
341 + key: null,
342 owner: null,
343 stack: gate(flag => flag.enableOwnerStacks)
344 ? ' in Object.<anonymous> (at **)'
@@ -2614,6 +2616,7 @@ describe('ReactFlight', () => {
2616 {
2617 name: 'ServerComponent',
2618 env: 'Server',
2619 + key: null,
2620 owner: null,
2621 stack: gate(flag => flag.enableOwnerStacks)
2622 ? ' in Object.<anonymous> (at **)'
@@ -2631,6 +2634,7 @@ describe('ReactFlight', () => {
2634 {
2635 name: 'ThirdPartyComponent',
2636 env: 'third-party',
2637 + key: null,
2638 owner: null,
2639 stack: gate(flag => flag.enableOwnerStacks)
2640 ? ' in Object.<anonymous> (at **)'
@@ -2645,6 +2649,7 @@ describe('ReactFlight', () => {
2649 {
2650 name: 'ThirdPartyLazyComponent',
2651 env: 'third-party',
2652 + key: null,
2653 owner: null,
2654 stack: gate(flag => flag.enableOwnerStacks)
2655 ? ' in myLazy (at **)\n in lazyInitializer (at **)'
@@ -2659,6 +2664,7 @@ describe('ReactFlight', () => {
2664 {
2665 name: 'ThirdPartyFragmentComponent',
2666 env: 'third-party',
2667 + key: '3',
2668 owner: null,
2669 stack: gate(flag => flag.enableOwnerStacks)
2670 ? ' in Object.<anonymous> (at **)'
@@ -2732,6 +2738,7 @@ describe('ReactFlight', () => {
2738 {
2739 name: 'ServerComponent',
2740 env: 'Server',
2741 + key: null,
2742 owner: null,
2743 stack: gate(flag => flag.enableOwnerStacks)
2744 ? ' in Object.<anonymous> (at **)'
@@ -2748,6 +2755,7 @@ describe('ReactFlight', () => {
2755 {
2756 name: 'Keyed',
2757 env: 'Server',
2758 + key: 'keyed',
2759 owner: null,
2760 stack: gate(flag => flag.enableOwnerStacks)
2761 ? ' in ServerComponent (at **)'
@@ -2763,6 +2771,7 @@ describe('ReactFlight', () => {
2771 {
2772 name: 'ThirdPartyAsyncIterableComponent',
2773 env: 'third-party',
2774 + key: null,
2775 owner: null,
2776 stack: gate(flag => flag.enableOwnerStacks)
2777 ? ' in Object.<anonymous> (at **)'
@@ -2920,6 +2929,7 @@ describe('ReactFlight', () => {
2929 {
2930 name: 'Component',
2931 env: 'A',
2932 + key: null,
2933 owner: null,
2934 stack: gate(flag => flag.enableOwnerStacks)
2935 ? ' in Object.<anonymous> (at **)'
@@ -3040,6 +3050,7 @@ describe('ReactFlight', () => {
3050 const greetInfo = {
3051 name: 'Greeting',
3052 env: 'Server',
3053 + key: null,
3054 owner: null,
3055 stack: gate(flag => flag.enableOwnerStacks)
3056 ? ' in Object.<anonymous> (at **)'
@@ -3050,6 +3061,7 @@ describe('ReactFlight', () => {
3061 {
3062 name: 'Container',
3063 env: 'Server',
3064 + key: null,
3065 owner: greetInfo,
3066 stack: gate(flag => flag.enableOwnerStacks)
3067 ? ' in Greeting (at **)'
packages/react-devtools-shared/src/__tests__/store-test.js
+8 -10
@@ -2439,7 +2439,7 @@ describe('Store', () => {
2439 });
2440
2441 // @reactVersion > 18.2
2442 - it('can reorder keyed components', async () => {
2442 + it('can reorder keyed server components', async () => {
2443 function ClientComponent({text}) {
2444 return <div>{text}</div>;
2445 }
@@ -2452,9 +2452,7 @@ describe('Store', () => {
2452 name: 'ServerComponent',
2453 env: 'Server',
2454 owner: null,
2455 - // TODO: Ideally the debug info should include the "key" too to
2456 - // preserve the virtual identity of the server component when
2457 - // reordered. Atm only the children of it gets reparented.
2455 + key: key,
2456 },
2457 ];
2458 return ServerPromise;
@@ -2468,11 +2466,11 @@ describe('Store', () => {
2466 expect(store).toMatchInlineSnapshot(`
2467 [root]
2468 ▾ <App>
2471 - ▾ <ServerComponent> [Server]
2469 + ▾ <ServerComponent key="A"> [Server]
2470 <ClientComponent key="A">
2473 - ▾ <ServerComponent> [Server]
2471 + ▾ <ServerComponent key="B"> [Server]
2472 <ClientComponent key="B">
2475 - ▾ <ServerComponent> [Server]
2473 + ▾ <ServerComponent key="C"> [Server]
2474 <ClientComponent key="C">
2475 `);
2476
@@ -2480,11 +2478,11 @@ describe('Store', () => {
2478 expect(store).toMatchInlineSnapshot(`
2479 [root]
2480 ▾ <App>
2483 - ▾ <ServerComponent> [Server]
2481 + ▾ <ServerComponent key="B"> [Server]
2482 <ClientComponent key="B">
2485 - ▾ <ServerComponent> [Server]
2483 + ▾ <ServerComponent key="A"> [Server]
2484 <ClientComponent key="A">
2487 - ▾ <ServerComponent> [Server]
2485 + ▾ <ServerComponent key="D"> [Server]
2486 <ClientComponent key="D">
2487 `);
2488 });
packages/react-devtools-shared/src/backend/fiber/renderer.js
+36 -15
@@ -2220,9 +2220,12 @@ export function attach(
2220
2221 const isProfilingSupported = false; // TODO: Support Tree Base Duration Based on Children.
2222
2223 - const key = null; // TODO: Track keys on ReactComponentInfo;
2224 - const env = instance.data.env;
2225 - let displayName = instance.data.name || '';
2223 + const componentInfo = instance.data;
2224 +
2225 + const key =
2226 + typeof componentInfo.key === 'string' ? componentInfo.key : null;
2227 + const env = componentInfo.env;
2228 + let displayName = componentInfo.name || '';
2229 if (typeof env === 'string') {
2230 // We model environment as an HoC name for now.
2231 displayName = env + '(' + displayName + ')';
@@ -2855,19 +2858,35 @@ export function attach(
2858 );
2859 }
2860 }
2858 - const firstRemainingChild = remainingReconcilingChildren;
2861 + // TODO: Find the best matching existing child based on the key if defined.
2862 +
2863 + let bestMatch = remainingReconcilingChildren;
2864 + if (componentInfo.key != null) {
2865 + // If there is a key try to find a matching key in the set.
2866 + bestMatch = remainingReconcilingChildren;
2867 + while (bestMatch !== null) {
2868 + if (
2869 + bestMatch.kind === VIRTUAL_INSTANCE &&
2870 + bestMatch.data.key === componentInfo.key
2871 + ) {
2872 + break;
2873 + }
2874 + bestMatch = bestMatch.nextSibling;
2875 + }
2876 + }
2877 if (
2860 - firstRemainingChild !== null &&
2861 - firstRemainingChild.kind === VIRTUAL_INSTANCE &&
2862 - firstRemainingChild.data.name === componentInfo.name &&
2863 - firstRemainingChild.data.env === componentInfo.env
2878 + bestMatch !== null &&
2879 + bestMatch.kind === VIRTUAL_INSTANCE &&
2880 + bestMatch.data.name === componentInfo.name &&
2881 + bestMatch.data.env === componentInfo.env &&
2882 + bestMatch.data.key === componentInfo.key
2883 ) {
2884 // If the previous children had a virtual instance in the same slot
2885 // with the same name, then we claim it and reuse it for this update.
2886 // Update it with the latest entry.
2868 - firstRemainingChild.data = componentInfo;
2869 - moveChild(firstRemainingChild);
2870 - previousVirtualInstance = firstRemainingChild;
2887 + bestMatch.data = componentInfo;
2888 + moveChild(bestMatch);
2889 + previousVirtualInstance = bestMatch;
2890 previousVirtualInstanceWasMount = false;
2891 } else {
2892 // Otherwise we create a new instance.
@@ -4321,11 +4340,13 @@ export function attach(
4340 ): InspectedElement | null {
4341 const canViewSource = false;
4342
4324 - const key = null; // TODO: Track keys on ReactComponentInfo;
4343 + const componentInfo = virtualInstance.data;
4344 + const key =
4345 + typeof componentInfo.key === 'string' ? componentInfo.key : null;
4346 const props = null; // TODO: Track props on ReactComponentInfo;
4347
4327 - const env = virtualInstance.data.env;
4328 - let displayName = virtualInstance.data.name || '';
4348 + const env = componentInfo.env;
4349 + let displayName = componentInfo.name || '';
4350 if (typeof env === 'string') {
4351 // We model environment as an HoC name for now.
4352 displayName = env + '(' + displayName + ')';
@@ -4384,7 +4405,7 @@ export function attach(
4405 // Does the component have legacy context attached to it.
4406 hasLegacyContext: false,
4407
4387 - key: key != null ? key : null,
4408 + key: key,
4409
4410 displayName: displayName,
4411 type: ElementTypeVirtual,
packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js
+2 -2
@@ -307,7 +307,7 @@ describe('ReactFlightDOMEdge', () => {
307
308 const serializedContent = await readResult(stream1);
309
310 - expect(serializedContent.length).toBeLessThan(410);
310 + expect(serializedContent.length).toBeLessThan(425);
311 expect(timesRendered).toBeLessThan(5);
312
313 const model = await ReactServerDOMClient.createFromReadableStream(stream2, {
@@ -374,7 +374,7 @@ describe('ReactFlightDOMEdge', () => {
374 const [stream1, stream2] = passThrough(stream).tee();
375
376 const serializedContent = await readResult(stream1);
377 - expect(serializedContent.length).toBeLessThan(__DEV__ ? 590 : 400);
377 + expect(serializedContent.length).toBeLessThan(__DEV__ ? 605 : 400);
378 expect(timesRendered).toBeLessThan(5);
379
380 const model = await ReactServerDOMClient.createFromReadableStream(stream2, {
packages/react-server/src/ReactFlightServer.js
+5
@@ -970,6 +970,7 @@ function callWithDebugContextInDEV<A, T>(
970 const componentDebugInfo: ReactComponentInfo = {
971 name: '',
972 env: task.environmentName,
973 + key: null,
974 owner: task.debugOwner,
975 };
976 if (enableOwnerStacks) {
@@ -1036,6 +1037,7 @@ function renderFunctionComponent<Props>(
1037 componentDebugInfo = ({
1038 name: componentName,
1039 env: componentEnv,
1040 + key: key,
1041 owner: task.debugOwner,
1042 }: ReactComponentInfo);
1043 if (enableOwnerStacks) {
@@ -1575,6 +1577,7 @@ function renderElement(
1577 const componentDebugInfo: ReactComponentInfo = {
1578 name: 'Fragment',
1579 env: (0, request.environmentName)(),
1580 + key: key,
1581 owner: task.debugOwner,
1582 stack:
1583 task.debugStack === null
@@ -2615,6 +2618,7 @@ function renderModelDestructive(
2618 > = {
2619 name: (value: any).name,
2620 env: (value: any).env,
2621 + key: (value: any).key,
2622 owner: (value: any).owner,
2623 };
2624 if (enableOwnerStacks) {
@@ -3287,6 +3291,7 @@ function renderConsoleValue(
3291 > = {
3292 name: (value: any).name,
3293 env: (value: any).env,
3294 + key: (value: any).key,
3295 owner: (value: any).owner,
3296 };
3297 if (enableOwnerStacks) {
packages/shared/ReactTypes.js
+1
@@ -190,6 +190,7 @@ export type ReactStackTrace = Array<ReactCallSite>;
190 export type ReactComponentInfo = {
191 +name?: string,
192 +env?: string,
193 + +key?: null | string,
194 +owner?: null | ReactComponentInfo,
195 +stack?: null | ReactStackTrace,
196 // Stashed Data for the Specific Execution Environment. Not part of the transport protocol