-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(server): add Standard Schema support #6079
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@fabian-hiller is attempting to deploy a commit to the trpc Team on Vercel. A member of the Team first needs to authorize it. |
| const result = await parser['~validate']({ value }); | ||
| if (result.issues) { | ||
| console.error('Schema issues:', result.issues); | ||
| throw new Error(result.issues[0].message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there not be a StandardError or something? Having a generic error here makes it impossible to do errorFormatter like this one? https://trpc.io/docs/server/error-formatting#adding-custom-formatting
Can we attach all the issues on Error.cause or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added our SchemaError utility class. Does this look okay to you? Or should we add SchemaError directly to the source code and not import it from @standard-schema/utils?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea no deps and mports. Just a way to revive the full error with all the issues instead of just the first one
KATT
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that y'all are converging at a standard way of doing things
We don't want third party dependencies in trpc though
packages/server/package.json
Outdated
| "dependencies": { | ||
| "@standard-schema/utils": "^0.1.0" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't want dependencies.
| "dependencies": { | |
| "@standard-schema/utils": "^0.1.0" | |
| } |
| if (result.issues) { | ||
| throw new SchemaError(result.issues); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If result.reason was an Error-type, we could've just rethrown here
But for now I think we gotta do something like
if (result.issues) {
throw new Error("Validation error", { cause: result.issues })
}For us, it would be a bit simpler if there was an actual Error here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As @juliusmarminge already pointed out, it'd be nice if it was something like
if (result.error) {
throw result.error
}tuples work well too
const [err, result] = await parser['~validate'](value);
if (err) {
throw err;
}
return resultThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think forcing schema libraries to use a specific error class to be compatible with Standard Schema is not a good idea, as it would unnecessarily compromise bundle size, performance (creating Error instances is expensive), and flexibility.
| '~types'?: { | ||
| input: TInput; | ||
| output: TParsedInput; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have some different wip-branches where we do infer errors too, does ~types also have an Error-subclass?
In the case of zod, we could infer that pretty easily in #6027 where we played with this, see L12 here:
trpc/packages/server/src/@trpc/parser.ts
Lines 2 to 15 in de219a3
| export interface ParserZodEsque<TInput, TParsedInput, TError> { | |
| _input: TInput; | |
| _output: TParsedInput; | |
| safeParseAsync: (input: unknown) => Promise< | |
| | { | |
| success: true; | |
| data: TParsedInput; | |
| } | |
| | { | |
| success: false; | |
| error: TError; | |
| } | |
| >; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the benefit of inferring the error type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think e.g. a form submission - it'd be nice to infer the right path when rendering the errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need the error class for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cause there's no way to get the type of an error cause, so if you'd to throw new Error("", { cause: issues }) you'd need a cast like (err.cause as SchemaIssues<TSchema>).fieldErrors.email for example. Or how would you infer here?
Would be nice if something strictly typed came out of the validate method directly as a built-in we could re-throw
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the example! I understand it much better now. I think this is also possible with Standard Schema. We know the input and output type, so we know all possible paths that can occur, and can transform them into an object of dot paths. I can provide example code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is basically to infer the input type from the schema. Standard Schema provides an InferInput util type for this. Then we could use a modified version of ts-essentials' Paths type, which converts the input type into a union of all possible paths. Finally, we pass this type to Record (or a custom mapped type) to construct the FieldErrors type.
type FieldErrors = Record<Paths<InferInput<TSchema>>, string[]>There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using an object might not be the best idea as errors could occur on e.g both foo and foo.bar simultaneously - an array with issues and paths is probably best for a generic lib - you can supply functions to turn it into an object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using an object might not be the best idea as errors could occur on e.g both foo and foo.bar simultaneously
You don't think this is a problem, or are you aware of an edge case? That's why I wrote that the Paths type needs to be modified. I can provide a raw implementation if you are interested.
an array with issues and paths is probably best for a generic lib
I completely agree. That's exactly what the Standard Schema does.
you can supply functions to turn it into an object
I have thought about providing such a function with @standard-schema/utils, but such an implementation requires a lot of thought to make it work for everyone. Some libraries are probably only interested in nested messages, because they assume (or know) that the root type is always valid. Others are also interested in errors related to the root schema. There may also be edge cases where a path cannot be converted to a dot path. Then there are libraries that need the value to be a string and others that need it to be a string[].
We are very interested in and grateful for feedback from libraries like tRPC to make the spec as perfect as possible.
|
Does this PR look good to you, or should I change or modify anything? |
|
how done is the spec? I notice it's sstill in beeta? Are you not open to returning some tuple-like thing from |
|
I have been working with Colin and others on the interface and we think the spec is already in good shape, but at the end of the day we are in beta and Standard Schema only works if we can get everyone on board. The idea of the beta is to get feedback like yours and we are more than happy to discuss things and improve the spec together. I am also available for a call to figure things out. |
|
We will probably release a new beta version of the spec this week. I keep you posted. |
|
I updated the Standard Schema version and improved the error handling. |
| }; | ||
| } | ||
|
|
||
| if ('~standard' in parser) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would probably be sensible to move this higher up. All of the existing validation libraries won't actually reach here.
Now that standard-schema exists, the onus should be on validation libraries to implement it, and trpc's docs should slowly move towards "just use any standard-schema lib". At that point, most users will expect the parser['~standard'].validate(...) method to be used over parser.parse(value) or whatever. Hopefully there being any differences would be very rare, but still I think better to prioritise standard-schema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that would be a breaking change though. since for example a validation failure on a zod schema would not throw ZodError when using ~standard.parse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why I put it at the end. Once Standard Schema is more established, tRPC may move it to the top when they release a new version.
|
Summarizing some discussion points for clarity/posterity, and because we'll be pointing to this PR as a model for other integrators 👍 1.
|
|
Thanks for the update and great summary! The PR looks pretty complete, what is there left to update here now? Is it to remove Would be lovely to get this PR merged when it's ready, as working with Not trying to put pressure, just curious. The changes here seems to make it much nicer to handle validation/parsing as well (without wrapping in the |
Valibot v1 supports Standard Schema. There should be no bugs. And tRPC should still support our old implementation. But I can look into it. Maybe I am wrong. Feel free to give me feedback. Maybe I missed something.
Exactly 😎 |
@fabian-hiller I don't want to hijack this PR, I described my issue in the section below. Click to showIssueThe issue I get is with // On the server
const querySchema = v.object({
q: v.optional(v.string(), '')
});
export const appRouter = router({
list: publicProcedure.input(v.parser(v.object({
q: v.optional(v.string(), '')
})))
});
export type QueryInput = v.InferInput<typeof querySchema>;
// ^ { q?: string | undefined; }
export type QueryOutput = v.InferOutput<typeof querySchema>;
// ^ { q: string; }// On the client
trpc.list.query({});
// ^ Property 'q' is missing in type '{}' but required in type '{ q: string; }'. ts(2345)
// The type here should be the same as `QueryInput`, but it is the same as `QueryOutput`The only issue I've had is when using Likely causeI haven't looked too close at the issue (don't know how tRPC manages to infer the output type), but it looks to me like the For now, I got it working correctly by wrapping publicProcedure
.input(compat(v.parser(querySchema)))
function compat<T extends v.Parser<any, any>>(parser: T): T & {
schema: {
_types?: {
input: v.InferInput<T['schema']>;
output: v.InferOutput<T['schema']>;
};
}
} {
return parser;
} |
|
Yes, what you say may be true. This should be fixed once Standard Schema is supported. It will also allow you to remove the |
KATT
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to see this PR get up to date and merged in general
If there's changes to the standard schema, I guess you can just add versioning too
The only tweak I'd want would be to vendor rather than importing the standard schema
| @@ -1,3 +1,5 @@ | |||
| import type { v1 } from '@standard-schema/spec'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please vendor this type instead of adding a dep
| import type { v1 } from '@standard-schema/spec'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. I created a standardSchema.ts file. Feel free to rename it or to move the code around.
WalkthroughThis pull request introduces significant updates to the Valibot library integration across multiple packages. The changes primarily involve updating the Valibot dependency from version 0.42.0 to 1.0.0-beta.14, modifying input and output validation approaches, and introducing a new Changes
Sequence DiagramsequenceDiagram
participant Client
participant Validator
participant StandardSchema
participant ErrorHandler
Client->>Validator: Send input data
Validator->>StandardSchema: Validate input
alt Validation Successful
StandardSchema-->>Validator: Return validated data
Validator-->>Client: Process request
else Validation Failed
StandardSchema->>ErrorHandler: Generate StandardSchemaV1Error
ErrorHandler-->>Client: Return detailed error issues
end
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/server/src/unstable-core-do-not-import/standardSchema.ts (3)
1-3: Consider removing or justifying ESLint rule disables.
If namespaces are crucial to the design, that's fine, but it's increasingly common to avoid TS namespaces in favor of modules. Removing theseeslint-disabledirectives might align with standard TypeScript best practices unless there’s a strong rationale to keep them.
5-7: Confirm the usage of'~standard'as a property key.
Using a tilde in the property name might create friction with certain tooling. Unless specifically required, consider a more conventional property name (e.g.,standardPropsor_standard).
75-90: Include more details in theError.message.
Currently, onlyissues[0]?.messageis used as the error message. If there are multiple validation failures, the user is unaware of the rest. Consider concatenating or summarizing all of the issue messages for better error visibility.constructor(issues: ReadonlyArray<StandardSchemaV1.Issue>) { - super(issues[0]?.message); + super( + issues.map((issue, idx) => `[${idx}] ${issue.message}`).join('\n'), + ); this.name = 'SchemaError'; this.issues = issues; }packages/server/src/unstable-core-do-not-import/parser.ts (1)
1-1: Use consistent import style.
Consider also importingSchemaErroras a named import to emphasize usage over type conversion.packages/tests/server/outputParser.test.ts (1)
5-6: Separate “valibot0” vs. “valibot1” imports at scale.
As more versions are introduced, consider grouping these imports clearly or placing them in different test files for better maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
packages/server/package.json(1 hunks)packages/server/src/unstable-core-do-not-import/parser.ts(4 hunks)packages/server/src/unstable-core-do-not-import/standardSchema.ts(1 hunks)packages/tests/package.json(1 hunks)packages/tests/server/outputParser.test.ts(5 hunks)packages/tests/server/validators.test.ts(5 hunks)www/docs/main/quickstart.mdx(2 hunks)www/docs/server/validators.md(1 hunks)www/package.json(1 hunks)
🔇 Additional comments (18)
packages/server/src/unstable-core-do-not-import/standardSchema.ts (1)
10-73: Interface definitions look coherent.
All exported types and interfaces are well-documented, which helps maintain clarity. These types provide a clear contract for validation results and error reporting.packages/server/src/unstable-core-do-not-import/parser.ts (3)
23-26: Good introduction ofParserStandardSchemaEsque.
This new type effectively expands parser capabilities for standard schemas.
58-59: Nicely extended union type.
IncludingParserStandardSchemaEsquein theParserWithInputOutputunion ensures wide compatibility without breaking existing parser logic.
121-130: Validate partial vs. full success scenario.
ThrowingSchemaErroron anyissuesis logical for standard usage, but consider if partial validation success is ever relevant and if so, handle it accordingly.Would you like to check references in the codebase to ensure partial success is not needed?
packages/tests/server/outputParser.test.ts (6)
108-114: valibot v0 test suite is consistent.
Assert that this test covers edge cases such asnull,undefined, or arrays if noteworthy. Otherwise, the coverage looks good.
135-144: Asynchronous test coverage is robust.
Performing both valid and invalid queries ensures that async validations are handled properly.
170-180: Transform test is well-structured.
Verifies that the transform pipeline correctly modifies the output.
207-232: valibot v1 tests.
Great addition to confirm the updated API under$v1. The changes appear to align with the new library structure.
234-267: Async test for valibot v1 is well-covered.
You might consider testing performance if these validations become more complex, but at this scale, it's fine.
269-302: Transformation logic for valibot v1 validated successfully.
This is a strong example of how transformations can be chained in the new version.packages/tests/server/validators.test.ts (2)
10-11: LGTM! Good separation of valibot versions.The split imports for
valibot0andvalibot1provide a clean way to test both versions simultaneously.
235-255: Comprehensive test coverage for valibot v1.The new test cases for valibot v1 maintain parity with v0 tests, covering:
- Basic validation
- Async validation
- Transform operations
Also applies to: 257-285, 287-324
packages/tests/package.json (1)
52-53: Verify stability of beta dependency.Using
[email protected]in tests could potentially lead to instability. Consider:
- Pinning to a specific beta version instead of using
^- Adding a comment explaining the dual-version strategy
www/package.json (1)
103-103: LGTM! Appropriate version pinning.The exact version pin to
1.0.0-beta.14in devDependencies is appropriate for the documentation website.packages/server/package.json (1)
144-144: Consider removing valibot dependency.Based on previous feedback ("We don't want dependencies"), consider:
- Moving valibot to peerDependencies if it's optional
- Removing it entirely if it's only needed for tests
If the dependency is necessary, the exact version pin to
1.0.0-beta.14should be explained in the PR description.www/docs/server/validators.md (1)
410-411: LGTM! The Valibot example has been updated correctly.The changes correctly reflect the new Valibot v1 syntax by removing the
v.parser()wrapper, which aligns with the Standard Schema support. The example now matches the simplified validation approach used by other schema libraries.www/docs/main/quickstart.mdx (2)
291-291: LGTM! Clear and accurate documentation for Valibot schema usage.The explanation of Valibot schema usage is concise and matches the implementation examples.
310-310: LGTM! Consistent with the new Valibot v1 syntax.The example correctly demonstrates the simplified validation syntax using
v.string()without the parser wrapper, maintaining consistency with other validation libraries shown in the guide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/server/src/unstable-core-do-not-import/parser.ts (1)
121-130: Consider enhancing error handling and type safety.The implementation is solid, but could benefit from these improvements:
if ('~standard' in parser) { // StandardSchemaEsque return async (value) => { + // Type narrowing for better type safety + const standardParser = parser['~standard'] as StandardSchemaV1<unknown, TType>; - const result = await parser['~standard'].validate(value); + const result = await standardParser.validate(value); if (result.issues) { - throw new SchemaError(result.issues); + throw new SchemaError(result.issues, 'Schema validation failed'); } return result.value; }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/server/package.json(1 hunks)packages/server/src/unstable-core-do-not-import/parser.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/package.json
🔇 Additional comments (3)
packages/server/src/unstable-core-do-not-import/parser.ts (3)
1-1: LGTM! Clean implementation of Standard Schema type.The import statement and type definition are well-structured and follow the existing naming conventions.
Also applies to: 23-26
58-59: LGTM! Placement of Standard Schema parser is appropriate.The placement of
ParserStandardSchemaEsqueat the end of the union aligns with the discussed strategy of treating Standard Schema as a fallback until it becomes more established.
121-130: Implementation aligns well with discussed requirements.The code successfully implements the agreed-upon approach from the PR discussions:
- Uses
SchemaErrorfor consistent error handling- Maintains proper placement in the parser chain
- Preserves type inference capabilities
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/server/src/vendor/standard-schema-v1/spec.ts (2)
1-8: Consider specifying a stable reference to the spec in the doc comment.
Pointing to a particular commit or release tag of thestandard-schemarepository can help ensure long-term consistency in case the main branch changes unexpectedly.
40-46: Consider including an optional error code field.
Providing an optional error code or category could help consumers handle different failure scenarios programmatically.packages/server/src/@trpc/server/index.ts (1)
40-40: Consider documenting the experimental status of StandardSchemaV1Error.The export looks good, but since it's from the unstable core module, consider adding a JSDoc comment to indicate its experimental status and potential for breaking changes.
+/** + * @experimental This feature is part of the Standard Schema v1 support and may have breaking changes. + */ StandardSchemaV1Error,packages/tests/server/errorFormatting.test.ts (1)
127-174: Consider adding more test cases for error handling edge cases.The test case effectively validates the basic error formatting functionality. However, consider adding tests for:
- Multiple validation errors
- Nested object validation errors
- Array validation errors
Here's an example of additional test cases:
test('custom error formatter with standard schema v1 (valibot) - multiple errors', async () => { const t = initTRPC.create({ errorFormatter: opts => ({ ...opts.shape, data: { ...opts.shape.data, standardSchemaV1Error: opts.error.cause instanceof StandardSchemaV1Error ? { issues: opts.error.cause.issues } : null, }, }), }); const input = v1.object({ age: v1.number(), items: v1.array(v1.string()), nested: v1.object({ value: v1.number(), }), }); const router = t.router({ test: t.procedure.input(input).query(({ input }) => input), }); const ctx = routerToServerAndClientNew(router); const err = await waitError( ctx.client.test.query({ age: '25', items: ['valid', 123], nested: { value: 'invalid' }, } as any), TRPCClientError, ); assert(err.data?.standardSchemaV1Error); expect(err.data.standardSchemaV1Error.issues).toHaveLength(3); });packages/tests/server/validators.test.ts (1)
Line range hint
139-160: Consider adding explicit version compatibility comments.The test case effectively validates valibot v0 functionality. Consider adding a comment to clarify the version compatibility requirements.
+/** + * @requires valibot ^0.42.0 + */ test('valibot v0', async () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
packages/server/src/@trpc/server/index.ts(1 hunks)packages/server/src/unstable-core-do-not-import.ts(1 hunks)packages/server/src/unstable-core-do-not-import/parser.ts(4 hunks)packages/server/src/vendor/standard-schema-v1/error.ts(1 hunks)packages/server/src/vendor/standard-schema-v1/spec.ts(1 hunks)packages/tests/server/errorFormatting.test.ts(2 hunks)packages/tests/server/validators.test.ts(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: E2E-tests (express-server)
- GitHub Check: e2e-legacy-node (next-prisma-todomvc, 20.x)
- GitHub Check: E2E-tests (express-minimal)
- GitHub Check: e2e-legacy-node (next-prisma-todomvc, 18.x)
- GitHub Check: E2E-tests (cloudflare-workers)
- GitHub Check: e2e-legacy-node (next-prisma-websockets-starter, 20.x)
- GitHub Check: E2E-tests (.test/ssg)
- GitHub Check: e2e-legacy-node (next-prisma-websockets-starter, 18.x)
- GitHub Check: E2E-tests (.test/internal-types-export)
- GitHub Check: e2e-legacy-node (next-prisma-starter, 20.x)
- GitHub Check: Release using pkg.pr.new
- GitHub Check: E2E-tests (.test/diagnostics-big-router)
- GitHub Check: Test a monorepo using built declaration files
- GitHub Check: e2e-legacy-node (next-prisma-starter, 18.x)
- GitHub Check: E2E-tests (.experimental/next-app-dir)
- GitHub Check: E2E-tests (Bun) (bun, ubuntu-latest)
- GitHub Check: E2E-tests (Deno) (deno-deploy)
- GitHub Check: test
- GitHub Check: Lint and auto-fix
🔇 Additional comments (15)
packages/server/src/vendor/standard-schema-v1/spec.ts (5)
9-14: Interface naming structure looks good.
Defining the~standardproperty underStandardSchemaV1aligns with the external spec. No immediate concerns here.
15-29:Propsinterface is well-organized.
Each property is clearly documented, and the usage of generics forInputandOutputis consistent with TypeScript best practices.
30-39: Result union is clear and explicit.
Splitting validation outcomes intoSuccessResultandFailureResultmakes the API easy to consume.
47-60: Issue and PathSegment interfaces align well with typical validation problem reports.
No concerns regarding the structure or naming—these interfaces are intuitive.
61-79: Type inference utilities are clearly defined.
EnsuringInferInputandInferOutputrevolve aroundNonNullablehelps reliably capture the declared types. Nicely done.packages/server/src/vendor/standard-schema-v1/error.ts (2)
1-2: Note about potential undefined message.
Ifissuesis empty,super(issues[0]?.message)will passundefinedtoError. You might consider a fallback message.
5-19:StandardSchemaV1Erroreffectively captures multiple issues.
Storing the array of issues is beneficial. Setting the error name to'SchemaError'is clear—no further changes needed.packages/server/src/unstable-core-do-not-import.ts (1)
43-44: Additional exports maintain consistent re-export patterns.
Exposing botherrorandspecaligns with the existing structure. Just keep in mind this file warns consumers not to import directly.packages/tests/server/errorFormatting.test.ts (1)
3-4: LGTM! Clean import organization.The imports are well-organized, grouping related imports together (trpc-related and validation libraries).
Also applies to: 9-10
packages/server/src/unstable-core-do-not-import/parser.ts (3)
1-2: LGTM! Clean import organization.The imports are well-organized, importing from the vendored Standard Schema v1 implementation.
24-27: LGTM! Clear type definition.The type definition for
ParserStandardSchemaEsqueis clear and follows the pattern of other parser types.
122-131: Consider moving the Standard Schema check higher in the priority list.Based on past review comments, once Standard Schema becomes more established, this check should be moved higher in the priority list. For now, keeping it at the end is appropriate to maintain backward compatibility.
Let's verify the impact of the Standard Schema validation order:
✅ Verification successful
Validation order change is safe to implement when needed
The current implementation uses mutually exclusive checks for each parser type, making the order safe to change without affecting functionality. The suggestion to move Standard Schema validation higher in priority is valid and can be implemented when Standard Schema becomes more established.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential conflicts with other validation methods. # Test: Search for validation libraries that might implement both Standard Schema and their own validation. rg -A 5 $'~standard\' in' .Length of output: 625
Script:
#!/bin/bash # Get the complete validation chain from parser.ts echo "=== Parser.ts content ===" cat ./packages/server/src/unstable-core-do-not-import/parser.ts echo -e "\n=== Documentation about Standard Schema ===" # Search for documentation about Standard Schema validation rg -i "standard.*schema" -A 5 -B 5 "README.md" || trueLength of output: 3784
Script:
#!/bin/bash # Search for tests and examples using StandardSchema echo "=== Tests using StandardSchema ===" rg -l "StandardSchema" ./packages/server/test/ || true echo -e "\n=== Usage examples of StandardSchema ===" rg -l "StandardSchema" ./packages/server/examples/ || trueLength of output: 419
packages/tests/server/validators.test.ts (3)
3-4: LGTM! Clean import organization.The imports are well-organized, clearly distinguishing between valibot v0 and v1.
Also applies to: 9-10
234-302: LGTM! Comprehensive error type testing.The test cases for valibot v1 are thorough, including validation of the
StandardSchemaV1Errorstructure and its issues.
304-371: LGTM! Thorough transformation testing.The test cases effectively validate async validation and transformation capabilities of valibot v1.
|
This pull request has been locked because we are very unlikely to see comments on closed issues. If you think, this PR is still necessary, create a new one with the same branch. Thank you. |


🎯 Changes
An union of schema library authors (Zod, Valibot, ArkType, ...) collaborating on a standard interface for schema libraries called Standard Schema. This simplifies implementation and prevents vendor lock-in. So far Valibot v1 supports this spec and Zod will follow soon. This PR adds support for all schema libraries that follow the v1 of this spec in the future.
✅ Checklist
Summary by CodeRabbit
Dependencies
valibotpackage to version1.0.0-beta.14across multiple packagesvalibotValidation
v.parser()wrapper in validation methodsError Handling
StandardSchemaV1Errorfor more structured error reportingDocumentation