Skip to content

Commit 72cc026

Browse files
SukkaWhuozhi
authored andcommitted
refactor: rewrite config schema in zod (#56383)
The PR supersedes the #53150, which is way too outdated, has way too many conflicts, and also heavily relies on GitHub Copilot (which makes the progress slow and tedious). The PR uses [`json-schema-to-zod`](https://github.com/StefanTerdell/json-schema-to-zod) (instead of the GitHub Copilot) to generate the zod schema, and manually replaces all generated `z.customRefine` with my hand-written zod schema. TODO: - [x] Convert schema - [x] Reduce `z.any()` usage - [x] Create human-readable errors from the `ZodError` - [x] Update test cases to reflect the latest error message ----- The benefit of using zod over ajv: - Easier maintenance: zod schema is straightforward to compose. - Better typescript support: config schema now strictly reflects the `NextConfig` type. - Smaller installation size: by replacing `ajv` and `@segment/ajv-human-errors` w/ `zod`, I am able to reduce installation size by 114 KiB. - Better Extension: the zod error message is easy to customize. ----- In the previous PR #56083, @feedthejim replaces `zod` w/ `superstruct`. `superstruct` is lightweight and fast, which makes it perfect for creating simple schemas for RSC payload. But, this also means `superstruct` has its limitations compared to `zod`: - `superstruct`'s syntax is different, and some utilities's usage is counter-intuitive: - `z.array(z.string()).gt(1)` vs `s.size(s.array(s.string()), 1)` - `z.numer().gt(1)` vs `s.size(s.number(), 1)`, `s.min(s.number(), 1)` - `z.boolean().optional().nullable()` vs `s.nullable(s.optional(z.boolean()))` - `superstruct` has weaker TypeScript support and worse DX compared to `zod` when composing huge schema: - `zod.ZodType + z.object()` can provide a more detailed type mismatch message on which specific property is the culprit, while `Describe + s.object()` provides almost no information at all. - `zod`'s schema is more powerful - `z.function()` supports `z.args()` and `z.returns()`, while `superstruct` only has `s.func()` - zod also has Promise type `z.promise()` and intersection type `z.and()` - `superstruct`'s error is harder to parse compared to `zod`'s `ZodError` So in the PR, I re-introduced `zod` for `next.config.js` validation.
1 parent 35e4539 commit 72cc026

File tree

11 files changed

+138
-60
lines changed

11 files changed

+138
-60
lines changed

packages/next-swc/crates/core/src/react_server_components.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,10 @@ impl<C: Comments> ReactServerComponents<C> {
344344
}
345345

346346
fn assert_server_graph(&self, imports: &[ModuleImports], module: &Module) {
347+
// If the
348+
if self.is_from_node_modules(&self.filepath) {
349+
return;
350+
}
347351
for import in imports {
348352
let source = import.source.0.clone();
349353
if self.invalid_server_imports.contains(&source) {
@@ -391,6 +395,9 @@ impl<C: Comments> ReactServerComponents<C> {
391395
}
392396

393397
fn assert_server_filename(&self, module: &Module) {
398+
if self.is_from_node_modules(&self.filepath) {
399+
return;
400+
}
394401
let is_error_file = Regex::new(r"[\\/]error\.(ts|js)x?$")
395402
.unwrap()
396403
.is_match(&self.filepath);
@@ -416,6 +423,9 @@ impl<C: Comments> ReactServerComponents<C> {
416423
}
417424

418425
fn assert_client_graph(&self, imports: &[ModuleImports]) {
426+
if self.is_from_node_modules(&self.filepath) {
427+
return;
428+
}
419429
for import in imports {
420430
let source = import.source.0.clone();
421431
if self.invalid_client_imports.contains(&source) {
@@ -432,6 +442,9 @@ impl<C: Comments> ReactServerComponents<C> {
432442
}
433443

434444
fn assert_invalid_api(&self, module: &Module, is_client_entry: bool) {
445+
if self.is_from_node_modules(&self.filepath) {
446+
return;
447+
}
435448
let is_layout_or_page = Regex::new(r"[\\/](page|layout)\.(ts|js)x?$")
436449
.unwrap()
437450
.is_match(&self.filepath);
@@ -562,6 +575,12 @@ impl<C: Comments> ReactServerComponents<C> {
562575
},
563576
);
564577
}
578+
579+
fn is_from_node_modules(&self, filepath: &str) -> bool {
580+
Regex::new(r"[\\/]node_modules[\\/]")
581+
.unwrap()
582+
.is_match(filepath)
583+
}
565584
}
566585

567586
pub fn server_components<C: Comments>(

test/development/acceptance-app/fixtures/rsc-build-errors/next.config.js

Lines changed: 0 additions & 1 deletion
This file was deleted.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
'use client'
2+
export default function page() {
3+
return 'page'
4+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
export const metadata = {
2+
title: 'Next.js',
3+
description: 'Generated by Next.js',
4+
}
5+
6+
export default function RootLayout({ children }) {
7+
return (
8+
<html lang="en">
9+
<body>{children}</body>
10+
</html>
11+
)
12+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export default function page() {
2+
return 'page'
3+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { useState } from 'react'
2+
3+
export function callClientApi() {
4+
return useState(0)
5+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "client-package",
3+
"exports": "./index.js"
4+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
'use client'
2+
3+
import { cookies } from 'next/headers'
4+
5+
export function callServerApi() {
6+
return cookies()
7+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "server-package",
3+
"exports": "./index.js"
4+
}

test/development/acceptance-app/rsc-build-errors.test.ts

Lines changed: 0 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -379,63 +379,4 @@ describe('Error overlay - RSC build errors', () => {
379379

380380
await cleanup()
381381
})
382-
383-
it('should show which import caused an error in node_modules', async () => {
384-
const { session, cleanup } = await sandbox(
385-
next,
386-
new Map([
387-
[
388-
'node_modules/client-package/module2.js',
389-
"import { useState } from 'react'",
390-
],
391-
['node_modules/client-package/module1.js', "import './module2.js'"],
392-
['node_modules/client-package/index.js', "import './module1.js'"],
393-
[
394-
'node_modules/client-package/package.json',
395-
outdent`
396-
{
397-
"name": "client-package",
398-
"version": "0.0.1"
399-
}
400-
`,
401-
],
402-
['app/Component.js', "import 'client-package'"],
403-
[
404-
'app/page.js',
405-
outdent`
406-
import './Component.js'
407-
export default function Page() {
408-
return <p>Hello world</p>
409-
}
410-
`,
411-
],
412-
])
413-
)
414-
415-
expect(await session.hasRedbox(true)).toBe(true)
416-
expect(
417-
next.normalizeTestDirContent(await session.getRedboxSource())
418-
).toMatchInlineSnapshot(
419-
next.normalizeSnapshot(`
420-
"./app/Component.js
421-
ReactServerComponentsError:
422-
423-
You're importing a component that needs useState. It only works in a Client Component but none of its parents are marked with \\"use client\\", so they're Server Components by default.
424-
Learn more: https://nextjs.org/docs/getting-started/react-essentials
425-
426-
,-[TEST_DIR/node_modules/client-package/module2.js:1:1]
427-
1 | import { useState } from 'react'
428-
: ^^^^^^^^
429-
\`----
430-
431-
The error was caused by importing 'client-package/index.js' in './app/Component.js'.
432-
433-
Maybe one of these should be marked as a client entry with \\"use client\\":
434-
./app/Component.js
435-
./app/page.js"
436-
`)
437-
)
438-
439-
await cleanup()
440-
})
441382
})
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import path from 'path'
2+
import { outdent } from 'outdent'
3+
import { FileRef, createNextDescribe } from 'e2e-utils'
4+
import {
5+
check,
6+
getRedboxDescription,
7+
hasRedbox,
8+
shouldRunTurboDevTest,
9+
} from 'next-test-utils'
10+
11+
createNextDescribe(
12+
'Error overlay - RSC runtime errors',
13+
{
14+
files: new FileRef(path.join(__dirname, 'fixtures', 'rsc-runtime-errors')),
15+
packageJson: {
16+
scripts: {
17+
setup: 'cp -r ./node_modules_bak/* ./node_modules',
18+
build: 'yarn setup && next build',
19+
dev: `yarn setup && next ${
20+
shouldRunTurboDevTest() ? 'dev --turbo' : 'dev'
21+
}`,
22+
start: 'next start',
23+
},
24+
},
25+
installCommand: 'yarn',
26+
startCommand: (global as any).isNextDev ? 'yarn dev' : 'yarn start',
27+
},
28+
({ next }) => {
29+
it('should show runtime errors if invalid client API from node_modules is executed', async () => {
30+
await next.patchFile(
31+
'app/server/page.js',
32+
outdent`
33+
import { callClientApi } from 'client-package'
34+
export default function Page() {
35+
callClientApi()
36+
return 'page'
37+
}
38+
`
39+
)
40+
41+
const browser = await next.browser('/server')
42+
43+
await check(
44+
async () => ((await hasRedbox(browser, true)) ? 'success' : 'fail'),
45+
/success/
46+
)
47+
const errorDescription = await getRedboxDescription(browser)
48+
49+
expect(errorDescription).toContain(
50+
`Error: useState only works in Client Components. Add the "use client" directive at the top of the file to use it. Read more: https://nextjs.org/docs/messages/react-client-hook-in-server-component`
51+
)
52+
})
53+
54+
it('should show runtime errors if invalid server API from node_modules is executed', async () => {
55+
await next.patchFile(
56+
'app/client/page.js',
57+
outdent`
58+
'use client'
59+
import { callServerApi } from 'server-package'
60+
export default function Page() {
61+
callServerApi()
62+
return 'page'
63+
}
64+
`
65+
)
66+
67+
const browser = await next.browser('/client')
68+
69+
await check(
70+
async () => ((await hasRedbox(browser, true)) ? 'success' : 'fail'),
71+
/success/
72+
)
73+
const errorDescription = await getRedboxDescription(browser)
74+
75+
expect(errorDescription).toContain(
76+
`Error: Invariant: cookies() expects to have requestAsyncStorage, none available.`
77+
)
78+
})
79+
}
80+
)

0 commit comments

Comments
 (0)