Skip to content

Commit

Permalink
Update on "[compiler] rfc: Include location information in identifier…
Browse files Browse the repository at this point in the history
…s and reactive scopes for debugging"

Summary: Using the change detection code to debug codebases that violate the rules of react is a lot easier when we have a source location corresponding to the value that has changed inappropriately. I didn't see an easy way to track that information in the existing data structures at the point of codegen, so this PR adds locations to identifiers and reactive scopes (the location of a reactive scope is the range of the locations of its included identifiers).

I'm interested if there's a better way to do this that I missed!

[ghstack-poisoned]
  • Loading branch information
mvitousek committed May 31, 2024
2 parents 819c414 + ca6f970 commit 8dfb52d
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 8dfb52d

Please sign in to comment.