@samitouri / QOS-React-1 / commits / 8bc81ca90f

Create a root task for every Flight response (#29673)

This lets any element created from the server, to bottom out with a client "owner" which is the creator of the Flight request. This could be a Server Action being invoked or a router. This is similar to how a client element bottoms out in the creator of the root element without an owner. E.g. where the root app element was created. Without this, we inherit the task of whatever is currently executing when we're parsing which can be misleading. Before: <img width="507" alt="Screenshot 2024-05-30 at 12 06 57 PM" src="https://github.com/facebook/react/assets/63648/e234db7e-67f7-404c-958a-5c5500ffdf1f"> After: <img width="555" alt="Screenshot 2024-05-30 at 4 59 04 PM" src="https://github.com/facebook/react/assets/63648/8ba6acb4-2ffd-49d4-bd44-08228ad4200e"> The before/after doesn't show much of a difference here but that's just because our Flight parsing loop is an async, which maybe it shouldn't be because it can be unnecessarily deep, and it creates a hidden line for every loop. That's what the `Promise.then` is. If the element is lazily initialized it's worse because we can end up in an unrelated render task as the owner - although that's its own problem.

Sebastian Markbåge committed May 31, 2024 at 13:54 UTC 8bc81ca90fb0afb3fe4a28d8931733a3e6fef994
1 file changed +34 -6
packages/react-client/src/ReactFlightClient.js
+34 -6
@@ -254,6 +254,7 @@ export type Response = {
254 _rowLength: number, // remaining bytes in the row. 0 indicates that we're looking for a newline.
255 _buffer: Array<Uint8Array>, // chunks received so far as part of this row
256 _tempRefs: void | TemporaryReferenceSet, // the set temporary references can be resolved from
257 + _debugRootTask?: null | ConsoleTask, // DEV-only
258 };
259
260 function readChunk<T>(chunk: SomeChunk<T>): T {
@@ -614,6 +615,7 @@ function getTaskName(type: mixed): string {
615 }
616
617 function createElement(
618 + response: Response,
619 type: mixed,
620 key: mixed,
621 props: mixed,
@@ -697,9 +699,15 @@ function createElement(
699 const callStack = buildFakeCallStack(stack, createTaskFn);
700 // This owner should ideally have already been initialized to avoid getting
701 // user stack frames on the stack.
700 - const ownerTask = owner === null ? null : initializeFakeTask(owner);
702 + const ownerTask =
703 + owner === null ? null : initializeFakeTask(response, owner);
704 if (ownerTask === null) {
702 - task = callStack();
705 + const rootTask = response._debugRootTask;
706 + if (rootTask != null) {
707 + task = rootTask.run(callStack);
708 + } else {
709 + task = callStack();
710 + }
711 } else {
712 task = ownerTask.run(callStack);
713 }
@@ -1106,6 +1114,7 @@ function parseModelTuple(
1114 // TODO: Consider having React just directly accept these arrays as elements.
1115 // Or even change the ReactElement type to be an array.
1116 return createElement(
1117 + response,
1118 tuple[1],
1119 tuple[2],
1120 tuple[3],
@@ -1149,6 +1158,14 @@ export function createResponse(
1158 _buffer: [],
1159 _tempRefs: temporaryReferences,
1160 };
1161 + if (supportsCreateTask) {
1162 + // Any stacks that appear on the server need to be rooted somehow on the client
1163 + // so we create a root Task for this response which will be the root owner for any
1164 + // elements created by the server. We use the "use server" string to indicate that
1165 + // this is where we enter the server from the client.
1166 + // TODO: Make this string configurable.
1167 + response._debugRootTask = (console: any).createTask('"use server"');
1168 + }
1169 // Don't inline this call because it causes closure to outline the call above.
1170 response._fromJSON = createFromJSONCallback(response);
1171 return response;
@@ -1730,6 +1747,7 @@ function buildFakeCallStack<T>(stack: string, innerCall: () => T): () => T {
1747 }
1748
1749 function initializeFakeTask(
1750 + response: Response,
1751 debugInfo: ReactComponentInfo | ReactAsyncInfo,
1752 ): null | ConsoleTask {
1753 if (taskCache === null || typeof debugInfo.stack !== 'string') {
@@ -1745,7 +1763,7 @@ function initializeFakeTask(
1763 const ownerTask =
1764 componentInfo.owner == null
1765 ? null
1748 - : initializeFakeTask(componentInfo.owner);
1766 + : initializeFakeTask(response, componentInfo.owner);
1767
1768 // eslint-disable-next-line react-internal/no-production-logging
1769 const createTaskFn = (console: any).createTask.bind(
@@ -1755,7 +1773,12 @@ function initializeFakeTask(
1773 const callStack = buildFakeCallStack(stack, createTaskFn);
1774
1775 if (ownerTask === null) {
1758 - return callStack();
1776 + const rootTask = response._debugRootTask;
1777 + if (rootTask != null) {
1778 + return rootTask.run(callStack);
1779 + } else {
1780 + return callStack();
1781 + }
1782 } else {
1783 return ownerTask.run(callStack);
1784 }
@@ -1776,7 +1799,7 @@ function resolveDebugInfo(
1799 // We eagerly initialize the fake task because this resolving happens outside any
1800 // render phase so we're not inside a user space stack at this point. If we waited
1801 // to initialize it when we need it, we might be inside user code.
1779 - initializeFakeTask(debugInfo);
1802 + initializeFakeTask(response, debugInfo);
1803 const chunk = getChunk(response, id);
1804 const chunkDebugInfo: ReactDebugInfo =
1805 chunk._debugInfo || (chunk._debugInfo = []);
@@ -1813,12 +1836,17 @@ function resolveConsoleEntry(
1836 printToConsole.bind(null, methodName, args, env),
1837 );
1838 if (owner != null) {
1816 - const task = initializeFakeTask(owner);
1839 + const task = initializeFakeTask(response, owner);
1840 if (task !== null) {
1841 task.run(callStack);
1842 return;
1843 }
1844 }
1845 + const rootTask = response._debugRootTask;
1846 + if (rootTask != null) {
1847 + rootTask.run(callStack);
1848 + return;
1849 + }
1850 callStack();
1851 }
1852