Skip to content

[Flight] Source Map Server Actions to their Server Location#30741

Merged
sebmarkbage merged 3 commits intofacebook:mainfrom
sebmarkbage:serverreferencelocation
Aug 18, 2024
Merged

[Flight] Source Map Server Actions to their Server Location#30741
sebmarkbage merged 3 commits intofacebook:mainfrom
sebmarkbage:serverreferencelocation

Conversation

@sebmarkbage
Copy link
Copy Markdown
Contributor

@sebmarkbage sebmarkbage commented Aug 17, 2024

This uses a similar technique to what we use to generate fake stack frames for server components. This generates an eval:ed wrapper function around the Server Reference proxy we create on the client. This wrapper function gets the original name of the action on the server and I also add a source map if findSourceMapURL is defined that points back to the source of the server function.

For "use server" on the server, there's no new API. It just uses the callsite of registerServerReference() on the Server. We can infer the function name from the actual function on the server and we already have the findSourceMapURL on the client receiving it.

For "use server" imported from the client, there's two new options added to createServerReference() (in addition to the optional encodeFormAction). These are only used in DEV mode. The findSourceMapURL option is the same one added in #29708. We need to pass this these references aren't created in the context of any specific request but globally. The other weird thing about this case is that this is actually a case where the compiled environment is the client so any source maps are the same as for the client layer, so the environment name here is just "Client".

  createServerReference(
    id: string,
    callServer: CallServerCallback,
    encodeFormAction?: EncodeFormActionCallback,
+   findSourceMapURL?: FindSourceMapURLCallback, // DEV-only
+   functionName?: string, // DEV-only
  )

The key is that we use the location of the registerServerReference()/createServerReference() call as the location of the function. A compiler can either emit those at the same locations as the original functions or use source maps to have those segments refer to the original location of the function (or in the case of a re-export the original location of the re-export is also a fine approximate). The compiled output must call these directly without a wrapper function because the wrapper adds a stack frame. I decided against complicated and fragile dev-only options to skip n number of frames that would just end up in prod code. The implementation just skips one frame - our own. Otherwise it'll just point all source mapping to the wrapper.

We don't have a "use server" imported from the client implementation in the reference implementation/fixture so it's a bit tricky to test that. In the case of CJS on the server, we just use a runtime instead of compiler so it's tricky to source map those appropriately. We can implement it for ESM on the server which is the main thing we're testing in the fixture. It's easier in a real implementation where all the compilation is just one pass. It's a little tricky since we have to parse and append to other source maps but I'd like to do that as a follow up. Or maybe that's just an exercise for the reader.

You can right click an action and click "Go to Definition".

Screenshot 2024-08-17 at 6 04 27 PM

For now they simply don't point to the right place but you can still jump to the right file in the fixture:

Screenshot 2024-08-17 at 5 58 40 PM

In Firefox/Safari given that the location doesn't exist in the source map yet, the browser refuses to open the file. Where as Chrome does nearest (last) line.

@sebmarkbage sebmarkbage requested review from eps1lon and gnoff August 17, 2024 22:35
@vercel
Copy link
Copy Markdown

vercel Bot commented Aug 17, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 18, 2024 4:25pm

@react-sizebot
Copy link
Copy Markdown

react-sizebot commented Aug 17, 2024

The size diff is too large to display in a single comment. The GitHub action for this pull request contains an artifact called 'sizebot-message.md' with the full message.

Generated by 🚫 dangerJS against b56c1d5

@sebmarkbage
Copy link
Copy Markdown
Contributor Author

Another thing to note is that useActionState / useState / useReducer all the return their own dispatcher function that doesn't point to anywhere useful and the Hooks inspecting doesn't show to the actual action/reducer. So there's no way to find the original action for those.

We should probably 1) expose those on the hooks inspection 2) do something clever with the dispatcher function so that it can point to whatever it wraps. E.g. it could itself be an eval like this or we could instrument it with something that at least React DevTools knows about and maybe you can expand in Chrome DevTools.

Comment thread packages/react-client/src/ReactFlightReplyClient.js
…ences

This ensures that it looks nicer when printed in debug tools and so that
you can inspect the function and see the server code location.

This assumes that the compiler generates source map locations that map the
location of the registerServerReference/createServerReference call to the
original function location or at least where the export happened.
@sebmarkbage sebmarkbage force-pushed the serverreferencelocation branch from 751b9b6 to b56c1d5 Compare August 18, 2024 16:23
@sebmarkbage sebmarkbage merged commit 6ebfd5b into facebook:main Aug 18, 2024
sebmarkbage added a commit that referenced this pull request Aug 21, 2024
Follow up to #30741.

This is just for the reference Webpack implementation.

If there is a source map associated with a Node ESM loader, we generate
new source map entries for every `registerServerReference` call.

To avoid messing too much with it, this doesn't rewrite the original
mappings. It just reads them while finding each of the exports in the
original mappings. We need to read all since whatever we append at the
end is relative. Then we just generate new appended entries at the end.

For the location I picked the location of the local name identifier.
Since that's the name of the function and that gives us a source map
name index. It means it jumps to the name rather than the beginning of
the function declaration. It could be made more clever like finding a
local function definition if it is reexported. We could also point to
the line/column of the function declaration rather than the identifier
but point to the name index of the identifier name.

Now jumping to definition works in the fixture.

<img width="574" alt="Screenshot 2024-08-20 at 2 49 07 PM"
src="https://github.com/user-attachments/assets/7710f0e6-2cee-4aad-8d4c-ae985f8289eb">

Unfortunately this technique doesn't seem to work in Firefox nor Safari.
They don't apply the source map for jumping to the definition.
sebmarkbage added a commit that referenced this pull request Aug 22, 2024
Follow up to #30741.

We need to keep the location of the action when it's bound so we can
still jump to it.
unstubbable added a commit to vercel/next.js that referenced this pull request Sep 27, 2024
For facebook/react#30741

This PR adds several additional parameters to the
`createServerReference` calls generated by the Server Reference SWC
transform, to later enable React to map server actions that are imported
into client components back to their server locations (dev mode only).

Currently the inner `findSourceMapURL` function is not yet implemented.

In a follow-up we will unwrap `registerServerReference` as well, which
is needed for server actions in the react server layer.

---------

Co-authored-by: Hendrik Liebau <mail@hendrik-liebau.de>
unstubbable added a commit to vercel/next.js that referenced this pull request Sep 27, 2024
In the same vein as #69190, where we already unwrapped
`createServerReference`, we now also unwrap `registerServerReference`,
which is required for React to select the right call stack frame when
generating source locations for server actions facebook/react#30741.

Whereas unwrapping of `createServerReference` was required for server
actions that are imported into client components, unwrapping
`registerServerReference` is needed for server actions that are passed
from server components to client components.

This does not fully enable the source mapping just yet. For this to work
end-to-end, the next step is to generate proper spans in the SWC
transform, which will be done in the next PR.
unstubbable added a commit to vercel/next.js that referenced this pull request Oct 2, 2024
In the same vein as #69190, where we already unwrapped
`createServerReference`, we now also unwrap `registerServerReference`,
which is required for React to select the right call stack frame when
generating source locations for server actions facebook/react#30741.

Whereas unwrapping of `createServerReference` was required for server
actions that are imported into client components, unwrapping
`registerServerReference` is needed for server actions that are passed
from server components to client components.

This does not fully enable the source mapping just yet. For this to work
end-to-end, the next step is to generate proper spans in the SWC
transform, which will be done in the next PR.
unstubbable added a commit to vercel/next.js that referenced this pull request Oct 4, 2024
In the same vein as #69190, where we already unwrapped
`createServerReference`, we now also unwrap `registerServerReference`,
which is required for React to select the right call stack frame when
generating source locations for server actions facebook/react#30741.

Whereas unwrapping of `createServerReference` was required for server
actions that are imported into client components, unwrapping
`registerServerReference` is needed for server actions that are passed
from server components to client components.

This does not fully enable the source mapping just yet. For this to work
end-to-end, the next step is to generate proper spans in the SWC
transform, which will be done in the next PR.
unstubbable added a commit to vercel/next.js that referenced this pull request Oct 4, 2024
In the same vein as #69190, where we already unwrapped `createServerReference`, we now also unwrap `registerServerReference`, which is required for React to select the right call stack frame when generating source locations for server actions (see facebook/react#30741).

Whereas unwrapping of `createServerReference` was required for server actions that are imported into client components, unwrapping `registerServerReference` is needed for server actions that are passed from server components to client components.

This does not fully enable the source mapping just yet. For this to work end-to-end, the next step is to generate proper spans in the SWC transform, which will be done in the next PR.
kdy1 pushed a commit to vercel/next.js that referenced this pull request Oct 10, 2024
In the same vein as #69190, where we already unwrapped `createServerReference`, we now also unwrap `registerServerReference`, which is required for React to select the right call stack frame when generating source locations for server actions (see facebook/react#30741).

Whereas unwrapping of `createServerReference` was required for server actions that are imported into client components, unwrapping `registerServerReference` is needed for server actions that are passed from server components to client components.

This does not fully enable the source mapping just yet. For this to work end-to-end, the next step is to generate proper spans in the SWC transform, which will be done in the next PR.
unstubbable added a commit to vercel/next.js that referenced this pull request Oct 11, 2024
…71042)

This PR adds support for showing the original server sources of server
components and server actions in the browser's developer tools.

To accomplish that, we're building on top of #69190, #70563, and #70564,
in which we prepared the proper generation of the source maps.

This PR completes the feature, by wiring up `findSourceMapURL` (see
facebook/react#30741) with a new dev server
middleware that serves those source map files to the browser.

Another change we had to make is disabling the dead-code elimination of
server actions in client bundles (see #70103), but only in development
mode. This optimization is primarily intended for production bundles, so
the change should not pose a problem.

With that, it's now possible to jump directly into the server sources
from the browser's dev tools, e.g. for component stacks of server logs
that are replayed in the browser, or when inspecting server action props
in the React DevTools.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants