Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] refactor: replace ajv with zod #53150

Closed
wants to merge 2 commits into from

Conversation

SukkaW
Copy link
Contributor

@SukkaW SukkaW commented Jul 25, 2023

This is an ongoing PR that replaces Next.js' ajv based config validation with the Zod (which is already a dependency of Next.js).

@SukkaW SukkaW marked this pull request as draft July 25, 2023 08:05
@ijjk
Copy link
Member

ijjk commented Jul 25, 2023

Allow CI Workflow Run

  • approve CI run for commit: 6510934

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@feedthejim feedthejim added the CI approved Approve running CI for fork label Jul 25, 2023
@feedthejim
Copy link
Contributor

One thing to note is that I'm working on bundling the parts of Next.js into split bundles for perf reasons. Zod is a deps, but only for the app router layer at the moment. This would bring it to the build/server layer I think, which is probably fine, just flagging.

Is AJV much worse than zod in terms of size/perf?

@SukkaW
Copy link
Contributor Author

SukkaW commented Jul 25, 2023

One thing to note is that I'm working on bundling the parts of Next.js into split bundles for perf reasons. Zod is a deps, but only for the app router layer at the moment. This would bring it to the build/server layer I think, which is probably fine, just flagging.

Is AJV much worse than zod in terms of size/perf?

My initial idea is to reduce the node_modules installation size, and having two libraries (both ajv and zod) doing the same validation thing is not ideal.

Besides, Zod's schema is much easier to write and maintain compares to ajv's schema, and the typescript support is also much better.

There are also potential downsizes. Unlike ajv, Zod doesn't have a pre-compile option. And we have to create human-readable messages from Zod's error as well (Currently, ajv has @segment/ajv-human-errors to print human-readable errors from ajv).

@belgattitude
Copy link
Contributor

Worth to share https://github.com/moltar/typescript-runtime-type-benchmarks for impact. Zod is probably the most convenient.

@SukkaW SukkaW closed this Oct 2, 2023
@SukkaW SukkaW deleted the replace-ajv-with-zod branch October 2, 2023 14:59
@SukkaW SukkaW restored the replace-ajv-with-zod branch October 3, 2023 11:25
@SukkaW SukkaW deleted the replace-ajv-with-zod branch October 3, 2023 11:26
kodiakhq bot pushed a commit that referenced this pull request Oct 5, 2023
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.
huozhi pushed a commit that referenced this pull request Oct 5, 2023
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.
huozhi pushed a commit that referenced this pull request Oct 5, 2023
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.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI approved Approve running CI for fork locked type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants