Skip to content

Commit d28356f

Browse files
hi-ogawaclaude
hi-ogawa
and
claude
authored
fix(rsc): handle transform errors before server hmr (#626)
Co-authored-by: Claude <[email protected]>
1 parent 73d457b commit d28356f

File tree

3 files changed

+257
-2
lines changed

3 files changed

+257
-2
lines changed

‎packages/plugin-rsc/e2e/syntax-error.test.ts

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
import { test, expect } from '@playwright/test'
2+
import { setupInlineFixture, useFixture } from './fixture'
3+
import { waitForHydration, expectNoReload } from './helper'
4+
5+
test.describe(() => {
6+
const root = 'examples/e2e/temp/syntax-error'
7+
8+
test.beforeAll(async () => {
9+
await setupInlineFixture({
10+
src: 'examples/starter',
11+
dest: root,
12+
files: {
13+
'src/root.tsx': /* tsx */ `
14+
import { TestSyntaxErrorClient } from './client.tsx'
15+
16+
export function Root() {
17+
return (
18+
<html lang="en">
19+
<head>
20+
<meta charSet="UTF-8" />
21+
</head>
22+
<body>
23+
<TestSyntaxErrorClient />
24+
<div data-testid="server-content">server:ok</div>
25+
</body>
26+
</html>
27+
)
28+
}
29+
`,
30+
'src/client.tsx': /* tsx */ `
31+
"use client";
32+
import { useState } from 'react'
33+
34+
export function TestSyntaxErrorClient() {
35+
const [count, setCount] = useState(0)
36+
37+
return (
38+
<div data-testid="client-syntax-ready">
39+
<button
40+
onClick={() => setCount(count + 1)}
41+
data-testid="client-counter"
42+
>
43+
Client Count: {count}
44+
</button>
45+
<div data-testid="client-content">client:ok</div>
46+
</div>
47+
)
48+
}
49+
`,
50+
},
51+
})
52+
})
53+
54+
test.describe(() => {
55+
const f = useFixture({ root, mode: 'dev' })
56+
57+
test('client hmr', async ({ page }) => {
58+
await page.goto(f.url())
59+
await waitForHydration(page)
60+
await using _ = await expectNoReload(page)
61+
await expect(page.getByTestId('client-content')).toHaveText('client:ok')
62+
63+
// Set client state to verify preservation after HMR
64+
await page.getByTestId('client-counter').click()
65+
await expect(page.getByTestId('client-counter')).toHaveText(
66+
'Client Count: 1',
67+
)
68+
69+
// add syntax error
70+
const editor = f.createEditor('src/client.tsx')
71+
editor.edit((s) =>
72+
s.replace(
73+
'<div data-testid="client-content">client:ok</div>',
74+
'<div data-testid="client-content">client:broken<</div>',
75+
),
76+
)
77+
await expect(page.locator('vite-error-overlay')).toBeVisible()
78+
79+
// fix syntax error
80+
await page.waitForTimeout(200)
81+
editor.edit((s) =>
82+
s.replace(
83+
'<div data-testid="client-content">client:broken<</div>',
84+
'<div data-testid="client-content">client:fixed</div>',
85+
),
86+
)
87+
await expect(page.locator('vite-error-overlay')).not.toBeVisible()
88+
await expect(page.getByTestId('client-syntax-ready')).toBeVisible()
89+
await expect(page.getByTestId('client-content')).toHaveText(
90+
'client:fixed',
91+
)
92+
await expect(page.getByTestId('client-counter')).toHaveText(
93+
'Client Count: 1',
94+
)
95+
})
96+
})
97+
98+
test.describe(() => {
99+
const f = useFixture({ root, mode: 'dev' })
100+
101+
test('server hmr', async ({ page }) => {
102+
await page.goto(f.url())
103+
await waitForHydration(page)
104+
await using _ = await expectNoReload(page)
105+
106+
await expect(page.getByTestId('server-content')).toHaveText('server:ok')
107+
108+
// Set client state to verify preservation during server HMR
109+
await page.getByTestId('client-counter').click()
110+
await expect(page.getByTestId('client-counter')).toHaveText(
111+
'Client Count: 1',
112+
)
113+
114+
// add syntax error
115+
const editor = f.createEditor('src/root.tsx')
116+
editor.edit((s) =>
117+
s.replace(
118+
'<div data-testid="server-content">server:ok</div>',
119+
'<div data-testid="server-content">server:broken<</div>',
120+
),
121+
)
122+
await expect(page.locator('vite-error-overlay')).toBeVisible()
123+
124+
// fix syntax error
125+
await page.waitForTimeout(200)
126+
editor.edit((s) =>
127+
s.replace(
128+
'<div data-testid="server-content">server:broken<</div>',
129+
'<div data-testid="server-content">server:fixed</div>',
130+
),
131+
)
132+
await expect(page.locator('vite-error-overlay')).not.toBeVisible()
133+
await expect(page.getByTestId('server-content')).toHaveText(
134+
'server:fixed',
135+
)
136+
await expect(page.getByTestId('client-counter')).toHaveText(
137+
'Client Count: 1',
138+
)
139+
})
140+
})
141+
142+
test.describe(() => {
143+
const f = useFixture({ root, mode: 'dev' })
144+
145+
test('client ssr', async ({ page }) => {
146+
// add syntax error
147+
const editor = f.createEditor('src/client.tsx')
148+
editor.edit((s) =>
149+
s.replace(
150+
'<div data-testid="client-content">client:ok</div>',
151+
'<div data-testid="client-content">client:broken<</div>',
152+
),
153+
)
154+
await page.goto(f.url())
155+
await expect(page.locator('body')).toContainText('src/client.tsx:15')
156+
157+
// fix syntax error
158+
await page.waitForTimeout(200)
159+
editor.edit((s) =>
160+
s.replace(
161+
'<div data-testid="client-content">client:broken<</div>',
162+
'<div data-testid="client-content">client:fixed</div>',
163+
),
164+
)
165+
await expect(async () => {
166+
await page.goto(f.url())
167+
await waitForHydration(page)
168+
await expect(page.getByTestId('client-content')).toHaveText(
169+
'client:fixed',
170+
)
171+
}).toPass()
172+
})
173+
})
174+
175+
test.describe(() => {
176+
const f = useFixture({ root, mode: 'dev' })
177+
178+
test('server ssr', async ({ page }) => {
179+
// add syntax error
180+
const editor = f.createEditor('src/root.tsx')
181+
editor.edit((s) =>
182+
s.replace(
183+
'<div data-testid="server-content">server:ok</div>',
184+
'<div data-testid="server-content">server:broken<</div>',
185+
),
186+
)
187+
await page.goto(f.url())
188+
await expect(page.locator('body')).toContainText('src/root.tsx:11')
189+
190+
// fix syntax error
191+
await page.waitForTimeout(200)
192+
editor.edit((s) =>
193+
s.replace(
194+
'<div data-testid="server-content">server:broken<</div>',
195+
'<div data-testid="server-content">server:fixed</div>',
196+
),
197+
)
198+
await expect(async () => {
199+
await page.goto(f.url())
200+
await waitForHydration(page)
201+
await expect(page.getByTestId('server-content')).toHaveText(
202+
'server:fixed',
203+
)
204+
}).toPass()
205+
})
206+
})
207+
})

‎packages/plugin-rsc/src/plugin.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import {
3131
} from './transforms'
3232
import { generateEncryptionKey, toBase64 } from './utils/encryption-utils'
3333
import { createRpcServer } from './utils/rpc'
34-
import { normalizeViteImportAnalysisUrl } from './vite-utils'
34+
import { normalizeViteImportAnalysisUrl, prepareError } from './vite-utils'
3535

3636
// state for build orchestration
3737
let serverReferences: Record<string, string> = {}
@@ -382,6 +382,20 @@ export default function vitePluginRsc(
382382

383383
if (!isInsideClientBoundary(ctx.modules)) {
384384
if (this.environment.name === 'rsc') {
385+
// transform js to surface syntax errors
386+
for (const mod of ctx.modules) {
387+
if (mod.type === 'js') {
388+
try {
389+
await this.environment.transformRequest(mod.url)
390+
} catch (e) {
391+
server.environments.client.hot.send({
392+
type: 'error',
393+
err: prepareError(e as any),
394+
})
395+
throw e
396+
}
397+
}
398+
}
385399
// server hmr
386400
ctx.server.environments.client.hot.send({
387401
type: 'custom',
@@ -773,6 +787,13 @@ window.__vite_plugin_react_preamble_installed__ = true;
773787
code += `
774788
const ssrCss = document.querySelectorAll("link[rel='stylesheet']");
775789
import.meta.hot.on("vite:beforeUpdate", () => ssrCss.forEach(node => node.remove()));
790+
`
791+
// close error overlay after syntax error is fixed and hmr is triggered.
792+
// https://github.com/vitejs/vite/blob/8033e5bf8d3ff43995d0620490ed8739c59171dd/packages/vite/src/client/client.ts#L318-L320
793+
code += `
794+
import.meta.hot.on("rsc:update", () => {
795+
document.querySelectorAll("vite-error-overlay").forEach((n) => n.close())
796+
});
776797
`
777798
return code
778799
},

‎packages/plugin-rsc/src/vite-utils.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
import fs from 'node:fs'
44
import path from 'node:path'
5-
import type { DevEnvironment, Rollup } from 'vite'
5+
import type { DevEnvironment, ErrorPayload, Rollup } from 'vite'
6+
import { stripVTControlCharacters as strip } from 'node:util'
67

78
export const VALID_ID_PREFIX = `/@id/`
89

@@ -125,3 +126,29 @@ export function normalizeViteImportAnalysisUrl(
125126

126127
return url
127128
}
129+
130+
// error formatting
131+
// https://github.com/vitejs/vite/blob/8033e5bf8d3ff43995d0620490ed8739c59171dd/packages/vite/src/node/server/middlewares/error.ts#L11
132+
133+
type RollupError = Rollup.RollupError
134+
135+
export function prepareError(err: Error | RollupError): ErrorPayload['err'] {
136+
// only copy the information we need and avoid serializing unnecessary
137+
// properties, since some errors may attach full objects (e.g. PostCSS)
138+
return {
139+
message: strip(err.message),
140+
stack: strip(cleanStack(err.stack || '')),
141+
id: (err as RollupError).id,
142+
frame: strip((err as RollupError).frame || ''),
143+
plugin: (err as RollupError).plugin,
144+
pluginCode: (err as RollupError).pluginCode?.toString(),
145+
loc: (err as RollupError).loc,
146+
}
147+
}
148+
149+
function cleanStack(stack: string) {
150+
return stack
151+
.split(/\n/)
152+
.filter((l) => /^\s*at/.test(l))
153+
.join('\n')
154+
}

0 commit comments

Comments
 (0)