Skip to content

Commit

Permalink
Create spans for createServerReference and registerServerReference (
Browse files Browse the repository at this point in the history
vercel#70564)

Creating proper source location spans for `createServerReference` and
`registerServerReference` is the next step in enabling source mapping of
server actions. With the added e2e test app, we can already verify that
this works by mocking `findSourceMapURL`.

Properly implementing `findSourceMapURL` will be the last missing piece
to complete the puzzle.

<img width="751" alt="server action go to definition"
src="https://github.com/user-attachments/assets/bfe26e92-c497-40d0-ab39-ba0febbd412f">

<img width="751" alt="server action source"
src="https://github.com/user-attachments/assets/daaecf4e-0bd7-44ab-b082-68a24cdf33f9">

<img width="1039" alt="source map viz server"
src="https://github.com/user-attachments/assets/9b2fc944-29e9-4ed4-82a9-8a27f82e913c">

<img width="1039" alt="source map viz client"
src="https://github.com/user-attachments/assets/7fc2f86c-8e74-49e6-be59-9edf5494eb57">
  • Loading branch information
unstubbable authored Oct 4, 2024
1 parent 8894bed commit a572740
Show file tree
Hide file tree
Showing 18 changed files with 262 additions and 70 deletions.
137 changes: 76 additions & 61 deletions crates/next-custom-transforms/src/transforms/server_actions.rs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// imported by the server.

export { callServer } from 'next/dist/client/app-call-server'
export { findSourceMapURL } from 'next/dist/client/app-find-source-map-url'

// A noop wrapper to let the Flight client create the server reference.
// See also: https://github.com/facebook/react/pull/26632
Expand All @@ -16,5 +17,3 @@ export const createServerReference = (
: // eslint-disable-next-line import/no-extraneous-dependencies
require('react-server-dom-webpack/client')) as typeof import('react-server-dom-webpack/client')
).createServerReference

export const findSourceMapURL = undefined
4 changes: 4 additions & 0 deletions packages/next/src/client/app-find-source-map-url.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// TODO: Will be implemented later.
export function findSourceMapURL(_filename: string): string | null {
return null
}
4 changes: 4 additions & 0 deletions packages/next/src/client/app-index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ import type { InitialRSCPayload } from '../server/app-render/types'
import { createInitialRouterState } from './components/router-reducer/create-initial-router-state'
import { MissingSlotContext } from '../shared/lib/app-router-context.shared-runtime'

// Importing from dist so that we can define an alias if needed.
import { findSourceMapURL } from 'next/dist/client/app-find-source-map-url'

/// <reference types="react-dom/experimental" />

const appElement: HTMLElement | Document | null = document
Expand Down Expand Up @@ -140,6 +143,7 @@ const readable = new ReadableStream({

const initialServerResponse = createFromReadableStream(readable, {
callServer,
findSourceMapURL,
})

// React overrides `.then` and doesn't return a new promise chain,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ import {
type NormalizedFlightData,
} from '../../flight-data-helpers'

// Importing from dist so that we can define an alias if needed.
import { findSourceMapURL } from 'next/dist/client/app-find-source-map-url'

export interface FetchServerResponseOptions {
readonly flightRouterState: FlightRouterState
readonly nextUrl: string | null
Expand Down Expand Up @@ -208,9 +211,7 @@ export async function fetchServerResponse(
// Handle the `fetch` readable stream that can be unwrapped by `React.use`.
const response: NavigationFlightResponse = await createFromFetch(
Promise.resolve(res),
{
callServer,
}
{ callServer, findSourceMapURL }
)

if (buildId !== response.b) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ import {
NEXT_URL,
RSC_CONTENT_TYPE_HEADER,
} from '../../app-router-headers'

// Importing from dist so that we can define an alias if needed.
import { findSourceMapURL } from 'next/dist/client/app-find-source-map-url'

// // eslint-disable-next-line import/no-extraneous-dependencies
// import { createFromFetch } from 'react-server-dom-webpack/client'
// // eslint-disable-next-line import/no-extraneous-dependencies
Expand Down Expand Up @@ -138,9 +142,7 @@ async function fetchServerAction(
if (contentType?.startsWith(RSC_CONTENT_TYPE_HEADER)) {
const response: ActionFlightResponse = await createFromFetch(
Promise.resolve(res),
{
callServer,
}
{ callServer, findSourceMapURL }
)

if (location) {
Expand Down
10 changes: 9 additions & 1 deletion packages/next/types/$$compiled.internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,15 @@ declare module 'next/dist/compiled/react-server-dom-turbopack/client.browser'
declare module 'next/dist/compiled/react-server-dom-turbopack/server.browser'
declare module 'next/dist/compiled/react-server-dom-turbopack/server.edge'
declare module 'next/dist/compiled/react-server-dom-turbopack/static.edge'
declare module 'next/dist/client/app-call-server'
declare module 'next/dist/client/app-call-server' {
export function callServer(
actionId: string,
actionArgs: unknown[]
): Promise<unknown>
}
declare module 'next/dist/client/app-find-source-map-url' {
export function findSourceMapURL(filename: string): string | null
}
declare module 'next/dist/compiled/react-dom/server'
declare module 'next/dist/compiled/react-dom/server.edge'
declare module 'next/dist/compiled/browserslist'
Expand Down
22 changes: 22 additions & 0 deletions test/e2e/app-dir/actions-simple/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
The main purpose of this end-to-end test app is to allow manual testing of
server action source mapping within the React DevTools.

Until we have properly implemented `findSourceMapURL` in Next.js, this demo only
works with Turbopack. This is because we can mock `findSourceMapURL` for the
test app, as Turbopack generates source map files, whereas Webpack uses
`eval-source-map`.

For client bundles, the source map files are served directly through
`/_next/static/chunks`, and for server bundles, the source map files are read
from disk and served through the `/source-maps-turbopack` route handler.

To check the source mapping of server actions, follow these steps:

1. Run `pnpm next dev --turbo test/e2e/app-dir/actions-simple`.
2. Go to [http://localhost:3000]() or [http://localhost:3000/client]().
3. Open the Components panel of the React DevTools.
4. Select the `Form` element.
5. In the props section, right-click on the `action` prop and select "Go to
definition" (sometimes it needs two tries).
6. You should end up in the Chrome DevTools Sources panel with the `actions.ts`
file open and the cursor at `foo()`.
28 changes: 28 additions & 0 deletions test/e2e/app-dir/actions-simple/actions-simple.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { nextTestSetup } from 'e2e-utils'
import { retry } from 'next-test-utils'

describe('actions-simple', () => {
const { next } = nextTestSetup({
files: __dirname,
})

it('should work with server actions passed to client components', async () => {
const browser = await next.browser('/')
expect(await browser.elementByCss('p').text()).toBe('initial')
await browser.elementByCss('button').click()

await retry(async () => {
expect(await browser.elementByCss('p').text()).toBe('result')
})
})

it('should work with server actions imported from client components', async () => {
const browser = await next.browser('/client')
expect(await browser.elementByCss('p').text()).toBe('initial')
await browser.elementByCss('button').click()

await retry(async () => {
expect(await browser.elementByCss('p').text()).toBe('result')
})
})
})
5 changes: 5 additions & 0 deletions test/e2e/app-dir/actions-simple/app/actions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use server'

export async function foo() {
return 'result'
}
15 changes: 15 additions & 0 deletions test/e2e/app-dir/actions-simple/app/client/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use client'

import { Form } from '../form'
import { foo } from '../actions'
import Link from 'next/link'

export default function Page() {
return (
<main>
<h1>client component page</h1>
<Form action={foo} />
<Link href="/">server component page</Link>
</main>
)
}
14 changes: 14 additions & 0 deletions test/e2e/app-dir/actions-simple/app/form.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use client'

import { useActionState } from 'react'

export function Form({ action }: { action: () => Promise<string> }) {
const [result, formAction] = useActionState(action, 'initial')

return (
<form action={formAction}>
<button>Submit</button>
<p>{result}</p>
</form>
)
}
8 changes: 8 additions & 0 deletions test/e2e/app-dir/actions-simple/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { ReactNode } from 'react'
export default function Root({ children }: { children: ReactNode }) {
return (
<html>
<body>{children}</body>
</html>
)
}
13 changes: 13 additions & 0 deletions test/e2e/app-dir/actions-simple/app/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { Form } from './form'
import { foo } from './actions'
import Link from 'next/link'

export default function Page() {
return (
<main>
<h1>server component page</h1>
<Form action={foo} />
<Link href="/client">client component page</Link>
</main>
)
}
18 changes: 18 additions & 0 deletions test/e2e/app-dir/actions-simple/app/source-maps-turbopack/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { readFile } from 'fs/promises'
import { NextRequest } from 'next/server'

// This is for mocking findSourceMapURL until we've implemented it properly.
export async function GET(request: NextRequest): Promise<Response> {
const filename = request.nextUrl.searchParams.get('filename')

try {
// It's not safe not to sanitize the query param, but it's just for a test.
const sourceMapContents = await readFile(`${filename}.map`)

return new Response(sourceMapContents)
} catch (error) {
console.error(error)
}

return new Response(null, { status: 404 })
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export function findSourceMapURL(filename: string): string | null {
if (filename.startsWith(`${document.location.origin}/_next/static`)) {
return `${filename}.map`
}

const url = new URL('/source-maps-turbopack', document.location.origin)
url.searchParams.set('filename', filename)

return url.href
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export function findSourceMapURL(_filename: string): string | null {
// TODO
return null
}
22 changes: 22 additions & 0 deletions test/e2e/app-dir/actions-simple/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* @type {import('next').NextConfig}
*/

const nextConfig = {
webpack(config) {
config.resolve.alias['next/dist/client/app-find-source-map-url'] =
require.resolve('./find-source-map-url-webpack-mock.ts')

return config
},
experimental: {
turbo: {
resolveAlias: {
'next/dist/client/app-find-source-map-url':
'./find-source-map-url-turbopack-mock.ts',
},
},
},
}

module.exports = nextConfig

0 comments on commit a572740

Please sign in to comment.