Skip to content

Commit

Permalink
Update on "[compiler] Option to always take the non-memo branch"
Browse files Browse the repository at this point in the history
Summary: This adds a debugging mode to the compiler that simply adds a `|| true` to the guard on all memoization blocks, which results in the generated code never using memoized values and always recomputing them. This is designed as a validation tool for the compiler's correctness--every program *should* behave exactly the same with this option enabled as it would with it disabled, and so any difference in behavior should be investigated as either a compiler bug or a pipeline issue.

(We add `|| true` rather than dropping the conditional block entirely because we still want to exercise the guard tests, in case the guards themselves are the source of an error, like reading a property from undefined in a guard.)

[ghstack-poisoned]
  • Loading branch information
mvitousek committed May 31, 2024
2 parents 94dd84d + 4328d55 commit a65fbef
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 6 deletions.
1 change: 1 addition & 0 deletions .github/ISSUE_TEMPLATE/config.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
blank_issues_enabled: false
contact_links:
- name: 📃 Documentation Issue
url: https://github.com/reactjs/react.dev/issues/new/choose
Expand Down
40 changes: 34 additions & 6 deletions packages/react-client/src/ReactFlightClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ export type Response = {
_rowLength: number, // remaining bytes in the row. 0 indicates that we're looking for a newline.
_buffer: Array<Uint8Array>, // chunks received so far as part of this row
_tempRefs: void | TemporaryReferenceSet, // the set temporary references can be resolved from
_debugRootTask?: null | ConsoleTask, // DEV-only
};

function readChunk<T>(chunk: SomeChunk<T>): T {
Expand Down Expand Up @@ -614,6 +615,7 @@ function getTaskName(type: mixed): string {
}

function createElement(
response: Response,
type: mixed,
key: mixed,
props: mixed,
Expand Down Expand Up @@ -697,9 +699,15 @@ function createElement(
const callStack = buildFakeCallStack(stack, createTaskFn);
// This owner should ideally have already been initialized to avoid getting
// user stack frames on the stack.
const ownerTask = owner === null ? null : initializeFakeTask(owner);
const ownerTask =
owner === null ? null : initializeFakeTask(response, owner);
if (ownerTask === null) {
task = callStack();
const rootTask = response._debugRootTask;
if (rootTask != null) {
task = rootTask.run(callStack);
} else {
task = callStack();
}
} else {
task = ownerTask.run(callStack);
}
Expand Down Expand Up @@ -1106,6 +1114,7 @@ function parseModelTuple(
// TODO: Consider having React just directly accept these arrays as elements.
// Or even change the ReactElement type to be an array.
return createElement(
response,
tuple[1],
tuple[2],
tuple[3],
Expand Down Expand Up @@ -1149,6 +1158,14 @@ export function createResponse(
_buffer: [],
_tempRefs: temporaryReferences,
};
if (supportsCreateTask) {
// Any stacks that appear on the server need to be rooted somehow on the client
// so we create a root Task for this response which will be the root owner for any
// elements created by the server. We use the "use server" string to indicate that
// this is where we enter the server from the client.
// TODO: Make this string configurable.
response._debugRootTask = (console: any).createTask('"use server"');
}
// Don't inline this call because it causes closure to outline the call above.
response._fromJSON = createFromJSONCallback(response);
return response;
Expand Down Expand Up @@ -1730,6 +1747,7 @@ function buildFakeCallStack<T>(stack: string, innerCall: () => T): () => T {
}

function initializeFakeTask(
response: Response,
debugInfo: ReactComponentInfo | ReactAsyncInfo,
): null | ConsoleTask {
if (taskCache === null || typeof debugInfo.stack !== 'string') {
Expand All @@ -1745,7 +1763,7 @@ function initializeFakeTask(
const ownerTask =
componentInfo.owner == null
? null
: initializeFakeTask(componentInfo.owner);
: initializeFakeTask(response, componentInfo.owner);

// eslint-disable-next-line react-internal/no-production-logging
const createTaskFn = (console: any).createTask.bind(
Expand All @@ -1755,7 +1773,12 @@ function initializeFakeTask(
const callStack = buildFakeCallStack(stack, createTaskFn);

if (ownerTask === null) {
return callStack();
const rootTask = response._debugRootTask;
if (rootTask != null) {
return rootTask.run(callStack);
} else {
return callStack();
}
} else {
return ownerTask.run(callStack);
}
Expand All @@ -1776,7 +1799,7 @@ function resolveDebugInfo(
// We eagerly initialize the fake task because this resolving happens outside any
// render phase so we're not inside a user space stack at this point. If we waited
// to initialize it when we need it, we might be inside user code.
initializeFakeTask(debugInfo);
initializeFakeTask(response, debugInfo);
const chunk = getChunk(response, id);
const chunkDebugInfo: ReactDebugInfo =
chunk._debugInfo || (chunk._debugInfo = []);
Expand Down Expand Up @@ -1813,12 +1836,17 @@ function resolveConsoleEntry(
printToConsole.bind(null, methodName, args, env),
);
if (owner != null) {
const task = initializeFakeTask(owner);
const task = initializeFakeTask(response, owner);
if (task !== null) {
task.run(callStack);
return;
}
}
const rootTask = response._debugRootTask;
if (rootTask != null) {
rootTask.run(callStack);
return;
}
callStack();
}

Expand Down

0 comments on commit a65fbef

Please sign in to comment.