Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request updates dependency versions and restructures the code for JSON schema conversion. The converter code is refactored from a procedural approach to an object-oriented design. Additionally, the tests for converting Zod schemas have been streamlined using parameterized strategies. New functions for handling custom JSON schemas and several new schema definitions (blob, file, regexp, and url) are introduced with accompanying test suites. The export structure has been reorganized, and a legacy file has been removed, consolidating utility functions into new, more modular files. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as ZodToJsonSchemaConverter
participant Z as Zod Schema
participant J as JSON Schema
U->>C: Call convert(Z, strategy)
C->>C: Validate schema properties & descriptions
C->>Z: Process Zod type conversion logic
C-->>J: Return [required flag, JSON schema structure]
U-->>J: Receives JSON Schema output
sequenceDiagram
participant U as User
participant F as customJsonSchema Function
participant D as Custom JSON Schema Def
participant G as getCustomJsonSchema Function
U->>F: Create schema with custom JSON schema
F->>D: Attach custom definition to schema
U->>G: Retrieve custom JSON schema (with chosen strategy)
G-->>U: Return custom JSON schema (if available)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (7)
🔇 Additional comments (7)
🪧 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 (
|
More templates
@orpc/client
@orpc/contract
@orpc/openapi-client
@orpc/openapi
@orpc/react-query
@orpc/server
@orpc/shared
@orpc/standard-server-fetch
@orpc/standard-server
@orpc/standard-server-node
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (14)
packages/zod/src/schemas/blob.test.ts (1)
4-11: Test coverage for blob schema looks good.The test thoroughly checks parsing of both Blob and File instances, error handling for invalid inputs, and verification of the custom Zod definition type. Consider adding tests for blobs with different content types or sizes to make the test suite more comprehensive.
packages/zod/src/schemas/url.test.ts (1)
4-10: Test coverage for URL schema looks solid.The test effectively verifies URL instance parsing, error handling for invalid inputs, and custom Zod definition. To enhance coverage, consider adding tests for different URL formats (including edge cases like relative URLs or URLs with special characters).
packages/zod/src/schemas/regexp.test.ts (1)
4-10: Test coverage for regexp schema is comprehensive.The test adequately covers RegExp instance parsing, error handling for invalid inputs with custom messages, and verification of the custom Zod definition. Consider adding tests with more complex regular expressions (e.g., with capture groups, flags, or special patterns) to strengthen test coverage.
packages/zod/src/schemas/file.test.ts (1)
16-28: Consider adding more MIME type validation test cases.While the current tests cover the basic scenarios, consider adding tests for more edge cases such as:
- Files with multiple MIME types
- Case sensitivity in MIME type matching
- Wildcard patterns beyond just the top-level type (e.g.,
image/pngspecifically)packages/zod/src/index.ts (1)
15-21: Consider documenting the 'oz' aggregator.
Although the aggregator object is handy for bundling schemas, it might benefit from a short docstring indicating its usage and purpose.packages/zod/src/custom-json-schema.ts (1)
27-57: Clone carefully when extending schemas.
ThecustomJsonSchema()function reusesschema.constructorto create a new schema object. While this is a standard approach in Zod, ensure that no hidden side effects occur in_defwhen cloning. If needed, consider using a dedicated cloning approach or Zod’s built-in.clone()if it suits your use case better.packages/zod/src/converter.test.ts (3)
11-247: Large test matrix is comprehensive but could be split for maintainability.
Thecasesarray thoroughly covers a range of string, number, and special Zod schemas. However, consider splitting these tests into thematic files or sub-suites (e.g., string-based schemas, numeric schemas, file/blob schemas) to improve comprehension and reduce file size.
193-195: Duplicate brand usage.
Lines 193–195 appear to duplicate the brand'CAT'logic from lines 189–191 with no functional difference. Consider removing or refactoring if intentional.
439-448: Valibot condition check is correct but untested.
The condition logic for third-party schemas (e.g.,valibot) is a simple boolean check. Consider adding an explicit negative test case to confirm it correctly rejects non-Zod targets.packages/zod/src/converter.ts (5)
10-33: Consider documenting the new option fields.
The addition ofunsupportedJsonSchemaandanyJsonSchemabroadens the usage scenarios. Consider updating or adding doc comments and examples to clarify their purpose and typical usage for maintainers and users.
50-92: Improve partial re-processing logic for description and custom JSON schemas.
Currently, the converter re-invokes itself with flags likeisHandledZodDescriptionandisHandledCustomJSONSchema. This double pass can be somewhat confusing. Consider handling description and custom schema detection in a single pass to reduce complexity.
93-192: Large switch-case for ZodString checks.
The thorough coverage of string constraints is impressive. However, the switch is quite long, making the code harder to read and maintain. Splitting this into helper methods (e.g.,convertStringCheck) could improve clarity, reduce code size, and promote testability.
569-601: handleCustomZodDef extension.
The logic here for handling custom definitions (blob, file, regexp, url) is very clear. As a next step, consider adding more test coverage to ensure you handle complex scenarios (e.g., custom mime types).Do you need help writing additional test cases for these custom definitions?
603-604: Helper naming consistency.
getZodTypeNameis succinct and follows a clear naming pattern. It might be more consistent to renamehandleCustomZodDeftogetCustomZodJsonSchemafor parallel naming, reflecting that it returns a JSON Schema object rather than “handling” in place.
📜 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 (17)
packages/zod/package.json(1 hunks)packages/zod/src/converter.test.ts(1 hunks)packages/zod/src/converter.ts(1 hunks)packages/zod/src/custom-json-schema.test-d.ts(1 hunks)packages/zod/src/custom-json-schema.test.ts(1 hunks)packages/zod/src/custom-json-schema.ts(1 hunks)packages/zod/src/index.ts(1 hunks)packages/zod/src/schemas.ts(0 hunks)packages/zod/src/schemas/base.ts(1 hunks)packages/zod/src/schemas/blob.test.ts(1 hunks)packages/zod/src/schemas/blob.ts(1 hunks)packages/zod/src/schemas/file.test.ts(1 hunks)packages/zod/src/schemas/file.ts(1 hunks)packages/zod/src/schemas/regexp.test.ts(1 hunks)packages/zod/src/schemas/regexp.ts(1 hunks)packages/zod/src/schemas/url.test.ts(1 hunks)packages/zod/src/schemas/url.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/zod/src/schemas.ts
🔇 Additional comments (29)
packages/zod/package.json (1)
47-50: Dependencies updated appropriately.The zod dependency has been updated to version "^3.24.2" and a new dev dependency "zod-to-json-schema" has been added. This aligns with the PR objective to rewrite @orpc/zod and likely supports the new JSON schema conversion functionality.
packages/zod/src/schemas/url.ts (1)
1-19: Well-structured URL validation schema with clear error messaging.The implementation of the
urlfunction successfully creates a Zod schema to validate URL objects, with appropriate error handling and custom type definition. The pattern follows a consistent approach that's likely used across other schema definitions.packages/zod/src/schemas/blob.ts (1)
1-20: Clean implementation of Blob validation schema.This implementation follows the same pattern as other schema validators in the codebase, with proper type annotations and error handling. The validator correctly checks for Blob instances and provides a clear error message when validation fails.
packages/zod/src/schemas/regexp.ts (1)
1-20: RegExp validation schema maintains consistent implementation pattern.The
regexpfunction follows the established pattern used in other schema validators, providing a clean and consistent API. The validation logic, error handling, and custom type definition are all implemented correctly.packages/zod/src/schemas/file.test.ts (1)
4-32: Comprehensive test suite for the file schema validator.The tests effectively cover both basic file validation and MIME type validation scenarios, including happy paths and various error cases. The assertions verify both the parsing behavior and the exact error messages.
packages/zod/src/custom-json-schema.test.ts (4)
1-3: Imports look good.
No issues detected with importing Zod and the custom schema utilities.
5-13: Thorough testing of the 'both' strategy.
The test case appropriately checks both valid and invalid inputs, ensuring error message correctness.
15-23: Coverage for 'input' strategy is well-handled.
This test case confirms that only input schemas carry the custom JSON schema, and respects error handling for invalid inputs.
25-34: Comprehensive validation of the 'output' strategy.
The error scenarios are properly tested, and the retrieval of the JSON schema is correctly asserted.packages/zod/src/index.ts (2)
1-6: Imports are structured and aligned with modular design.
Each newly added schema andcustomJsonSchemaimport is clearly organized.
8-14: Export structure is clear and consistent.
Re-exporting the new schemas and thecustom-json-schemamodule helps maintain a centralized export model.packages/zod/src/custom-json-schema.test-d.ts (3)
5-25: Correct usage of @ts-expect-error for invalid examples in 'both' strategy.
The tests accurately validate expected behavior for type failures, ensuring compile-time type checks are enforced.
27-43: Good coverage for 'input' strategy type checks.
This suite confirms correct handling of input-only schemas, including invalid input examples.
45-62: Validations for 'output' strategy are comprehensive.
The use of type transformation checks and @ts-expect-error provides confidence that output schemas are enforced correctly.packages/zod/src/schemas/file.ts (2)
1-4: Dependency imports are appropriate.
Importingwildcard-matchand Zod utilities is consistent with the new schema’s requirements.
5-23: Custom file schema is well-structured.
Use ofcustomensures only valid File objects pass. ThecomposeParamsusage fosters clear error messages.packages/zod/src/custom-json-schema.ts (3)
1-3: Consider verifying the JSON schema dependency setup.
These import statements look correct and consistent, but ensure that"json-schema-typed/draft-2020-12"is properly included in the project dependencies and aligns with the intended Zod version.
4-7: Symbols approach is clear and modular.
Using dedicated symbols to store schemas per strategy is a sound design choice. It ensures no collisions and keeps the definitions neatly scoped.
8-25: Validate Zod definitions before casting.
While thegetCustomJsonSchema()function is well-structured, consider validating thatdefis a valid Zod definition before casting toExclude<JSONSchema, boolean>. This could prevent runtime errors ifdefis malformed or overridden.packages/zod/src/schemas/base.ts (3)
1-6: Extend error parameters thoughtfully.
CustomParamsextendsCustomErrorParamswith an optionalfatalproperty. Ensure that usage offatalis consistently handled across the codebase and tested for edge cases (e.g., a truly fatal validation scenario).
7-22: Symbols for custom definitions are straightforward.
Using a dedicated symbol forORPC_CUSTOM_ZOD_DEFand associated utility functions (setCustomZodDef,getCustomZodDef) is tidy and helps maintain separation of standard Zod definitions from your custom additions.
24-55: Check for potential message collisions incomposeParams().
composeParams()merges a default message with either a user-supplied function, object, or string. If the user-supplied object also includesmessage, it will override the default. Confirm this behavior is intentional and documented.packages/zod/src/converter.test.ts (2)
1-10: Imports are logically structured.
The combination of imports fromjson-schema-typed,zod,zod-to-json-schema, and local modules is clearly organized, making the test dependencies easy to follow.
249-437: Robust coverage for input/output strategies.
Parameterizing over['input', 'output']ensures comprehensive coverage. Make sure to revisit the commented-out union, undefined, and transform cases to either remove them if they’re obsolete or re-enable them to maintain completeness.packages/zod/src/converter.ts (5)
35-48: Class-based approach looks good!
Switching from a procedural to a class-based design (ZodToJsonSchemaConverter) enhances maintainability and testability. The constructor clearly defines defaults and ensures sensible fallback values.
242-245: Questionable handling of ZodNaN as null type.
MappingZodNaNto{ type: 'null' }may be semantically off. If the library’s semantics for “NaN” are more akin to an invalid or special numeric variant, consider returning theanyJsonSchemaor specifying a unique approach.Would you like to confirm if
ZodNaNis truly meant to be interpreted as anullin JSON Schema?
264-295: Correct optional item handling in arrays.
The approach of wrapping item schemas withanyOf: [{ type: 'null' }, itemJson]is a neat solution to allow null items. Ensure that all call sites expect this behavior and that “allow null array items” is consistent with the rest of the codebase’s design.
434-459: Union/discriminated union approach looks consistent.
Good use ofanyOfto capture union types. One subtlety: if a sub-schema is unsupported, you skip it (itemJson !== this.unsupportedJsonSchema). This can change the meaning of the final “anyOf”. Double-check that discarding an unsupported sub-schema is truly desired.
461-478: Intersection’s required-boolean logic.
Currently, you're returning[required.length !== allOf.length, { allOf }]. This setsrequired = trueonly if both sides are strictly required. Confirm that this meets expectations for optional vs. required merges in Zod intersection types, especially if a single side is optional.
| return Object.assign(schema, { | ||
| type: ( | ||
| mimeType: string, | ||
| params?: string | CustomParams | ((input: unknown) => CustomParams), | ||
| ) => { | ||
| const isMatch = wcmatch(mimeType) | ||
|
|
||
| const refinedSchema = schema.refine( | ||
| val => isMatch(val.type.split(';')[0]!), | ||
| composeParams<File>( | ||
| val => `Expected a file of type ${mimeType} but got a file of type ${val.type || 'unknown'}`, | ||
| params, | ||
| ), | ||
| ) | ||
|
|
||
| setCustomZodDef(refinedSchema._def, { type: 'file', mimeType }) | ||
|
|
||
| return refinedSchema | ||
| }, | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle edge cases in MIME type checking.
Splitting val.type could fail if val.type is empty or undefined. Consider a fallback check before calling .split(';')[0]!.
Example fix:
- val => isMatch(val.type.split(';')[0]!)
+ val => val?.type
+ ? isMatch(val.type.split(';')[0] ?? '')
+ : falseThis ensures that the code won’t throw unexpectedly for malformed or missing file types.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return Object.assign(schema, { | |
| type: ( | |
| mimeType: string, | |
| params?: string | CustomParams | ((input: unknown) => CustomParams), | |
| ) => { | |
| const isMatch = wcmatch(mimeType) | |
| const refinedSchema = schema.refine( | |
| val => isMatch(val.type.split(';')[0]!), | |
| composeParams<File>( | |
| val => `Expected a file of type ${mimeType} but got a file of type ${val.type || 'unknown'}`, | |
| params, | |
| ), | |
| ) | |
| setCustomZodDef(refinedSchema._def, { type: 'file', mimeType }) | |
| return refinedSchema | |
| }, | |
| }) | |
| return Object.assign(schema, { | |
| type: ( | |
| mimeType: string, | |
| params?: string | CustomParams | ((input: unknown) => CustomParams), | |
| ) => { | |
| const isMatch = wcmatch(mimeType) | |
| const refinedSchema = schema.refine( | |
| - val => isMatch(val.type.split(';')[0]!), | |
| + val => val?.type | |
| + ? isMatch(val.type.split(';')[0] ?? '') | |
| + : false, | |
| composeParams<File>( | |
| val => `Expected a file of type ${mimeType} but got a file of type ${val.type || 'unknown'}`, | |
| params, | |
| ), | |
| ) | |
| setCustomZodDef(refinedSchema._def, { type: 'file', mimeType }) | |
| return refinedSchema | |
| }, | |
| }) |
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
packages/openapi/src/openapi-generator.test.ts (1)
29-39: Embrace new return structure by verifying the boolean usage.Your test now mocks
[true, ...]. Ensure the boolean portion accurately reflects the converter’s condition. Ifconvertcan return[false, ...], consider adding a test case for that scenario as well.packages/openapi/src/openapi-generator.ts (2)
135-135: Consider the unused boolean in converter results.Here, only the second element of
(boolean, JSONSchema)is used. Ifconvertreturns[false, ...], the code might still proceed. Either ensure prior validation ofcondition()or handle thefalsecase to avoid incorrect assumptions.Also applies to: 145-145
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 135-135: packages/openapi/src/openapi-generator.ts#L135
Added line #L135 was not covered by tests
216-216: Duplicate ignoring offalseoutcome.These lines replicate the same pattern of ignoring the boolean result. Verify whether the code must guard against
[false, ...]returns or confirm the condition can never be false in this context.Also applies to: 226-226
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 216-216: packages/openapi/src/openapi-generator.ts#L216
Added line #L216 was not covered by testspackages/openapi/src/schema-converter.ts (3)
7-8: Add clarifying doc comments.
While the tuple return type is effective, consider adding doc comments to clarify what therequiredboolean indicates in different scenarios.
10-11: Optional explanation for condition logic.
Theconditionmethod is straightforward, but adding a short doc comment on expected usage or known corner cases could help future maintainers.
28-28: Add test coverage for the fallback case.
The fallback[false, {}]branch is not covered by tests. Please verify that this scenario is tested to ensure coverage and correctness.Here’s a sample diff to introduce a fallback test scenario (e.g., in an existing test file):
+ test('Fallback converter returns empty schema', () => { + // Arrange: Provide a schema that no converter handles + const dummySchema: any = { type: 'unknown' } + // Act + const converter = new CompositeSchemaConverter([]) + const [required, result] = converter.convert(dummySchema, 'input') + // Assert + expect(required).toBe(false) + expect(result).toEqual({}) + })🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 28-28: packages/openapi/src/schema-converter.ts#L28
Added line #L28 was not covered by testspackages/zod/src/converter.test.ts (1)
1-616: Impressive Test Suite with Minor Verification SuggestionsThis new file provides thorough coverage for various Zod features—including advanced string checks, arrays, unions, and transformations—nicely ensuring correctness across multiple scenarios.
Emoji Regex
The regex used inz.string().emoji()(lines 66–68) includes Unicode property escapes like\p{Extended_Pictographic}, which typically require the "u" (Unicode) flag in JS regex engines. Consider verifying that downstream usage (or bundlers/transpilers) correctly supports the "u" flag for robust matching.Dynamic Import of valibot
In the test starting at lines 607–616, the code dynamically importsvalibotand confirms thatconverter.condition()returns false for avalibotschema. Although this verifies behavior with a different library, be mindful that dynamic imports can cause runtime overhead. If performance is critical, consider deferring or avoiding such imports in performance-sensitive tests.No critical issues are apparent beyond these points. The parameterized testing approach and coverage of diverse Zod capabilities look well organized and maintainable.
packages/zod/src/coercer.ts (1)
4-406: Good Coercion Logic with a Strict Date CheckOverall, the refactored
zodCoerceInternalfunction and its supportingsafeTo*functions are a clean, well-organized approach to converting string inputs to typed values. A couple of clarifications:
Strict ISO Date Matching
The check insafeToDate()(lines 397–405) enforcesdate.toISOString().startsWith(value). This can be too strict if the input omits trailing segments or differs in time zone format. If intended, no change is needed, but consider whether partial ISO strings or local time zone offsets should also be valid.Union Coercion
In theZodUnion/ZodDiscriminatedUnionblock (lines 241–269), iterating all union options and picking the one with the fewest parse errors is an interesting approach to fallback logic. This might result in non-intuitive resolutions if multiple branches partially match. If you want stricter matching, consider short-circuiting as soon as you find a valid parse.These minor concerns aside, the plugin-oriented design and fallback logic for unsupported or uncoercible input appear well-reasoned.
📜 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 (22)
apps/content/docs/openapi/plugins/zod-smart-coercion.md(0 hunks)eslint.config.js(1 hunks)packages/openapi/src/openapi-content-builder.ts(1 hunks)packages/openapi/src/openapi-generator.test.ts(7 hunks)packages/openapi/src/openapi-generator.ts(8 hunks)packages/openapi/src/openapi-input-structure-parser.ts(4 hunks)packages/openapi/src/openapi-output-structure-parser.ts(4 hunks)packages/openapi/src/openapi-parameters-builder.ts(2 hunks)packages/openapi/src/schema-converter.ts(1 hunks)packages/openapi/src/schema-utils.ts(4 hunks)packages/openapi/src/schema.ts(2 hunks)packages/zod/package.json(1 hunks)packages/zod/src/coercer.test.ts(1 hunks)packages/zod/src/coercer.ts(2 hunks)packages/zod/src/converter.test.ts(1 hunks)packages/zod/src/converter.ts(1 hunks)packages/zod/src/custom-json-schema.ts(1 hunks)packages/zod/src/index.ts(1 hunks)packages/zod/tests/coercer.test.ts(0 hunks)packages/zod/tests/custom.test.ts(0 hunks)packages/zod/tests/openapi.test-d.ts(0 hunks)packages/zod/tests/openapi.test.ts(0 hunks)
💤 Files with no reviewable changes (5)
- apps/content/docs/openapi/plugins/zod-smart-coercion.md
- packages/zod/tests/openapi.test-d.ts
- packages/zod/tests/custom.test.ts
- packages/zod/tests/openapi.test.ts
- packages/zod/tests/coercer.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/zod/package.json
🧰 Additional context used
🪛 GitHub Check: codecov/patch
packages/openapi/src/openapi-generator.ts
[warning] 135-135: packages/openapi/src/openapi-generator.ts#L135
Added line #L135 was not covered by tests
[warning] 145-145: packages/openapi/src/openapi-generator.ts#L145
Added line #L145 was not covered by tests
[warning] 216-216: packages/openapi/src/openapi-generator.ts#L216
Added line #L216 was not covered by tests
[warning] 226-226: packages/openapi/src/openapi-generator.ts#L226
Added line #L226 was not covered by tests
packages/openapi/src/openapi-input-structure-parser.ts
[warning] 91-91: packages/openapi/src/openapi-input-structure-parser.ts#L91
Added line #L91 was not covered by tests
packages/openapi/src/schema-converter.ts
[warning] 28-28: packages/openapi/src/schema-converter.ts#L28
Added line #L28 was not covered by tests
🔇 Additional comments (46)
packages/openapi/src/openapi-generator.test.ts (4)
103-150: Validate typed schema with thesatisfies JSONSchemaoperator.This usage is a neat way to ensure correctness of the mocked schema structure at compile-time. The logic seems cohesive, and no immediate issues stand out.
153-174: Mirror the boolean-based mock.This block also returns
[true, {...}]. If the converter could return[false, ...], confirm that the unhandled branch doesn’t break functionality. Adding a quick test for the false-condition path might be valuable.
272-272: Add test coverage for mocked error flow.Using the same mocked return for error coverage is fine, but the static analysis indicates lack of coverage. Consider adding a test that verifies the handling of this line specifically.
327-327: Expand error scenario testing.Same logic here: we see the repeated pattern of
[true, { description: '__mocked__' }]with minimal coverage. Adding another test path for different boolean outcomes or varied schemas would strengthen reliability.packages/openapi/src/openapi-content-builder.ts (2)
11-11: Move to broader JSONSchema type.Switching from a nested
JSONSchema.JSONSchemareference to the top-levelJSONSchematype makes the code more flexible and aligns with recent refactoring. No concerns detected.
16-19: Explicit typing for file schemas.Casting
matchesto(JSONSchema & { type: 'string'; contentMediaType: string })[]clarifies that these branches handle file-based schemas. This is clean and maintainable.packages/openapi/src/openapi-generator.ts (2)
5-5: AdoptConditionalSchemaConverterfor modular schema handling.Replacing
SchemaConverter[]withConditionalSchemaConverter[]clearly expresses that converters execute only when certain conditions are met. This design is more modular and explicit.Also applies to: 28-28
284-284: Refine error schema building and usage ofconvert.Declaring
schemas: JSONSchema[]and stating} satisfies JSONSchemaenforces strong typing—good practice. However, ignoring the boolean part at line 299 might mask a failure path ifconvertyields[false, ...].Also applies to: 296-296, 299-299
packages/openapi/src/openapi-parameters-builder.ts (2)
8-8: Type import simplification from JSONSchema.JSONSchema to JSONSchemaThis change aligns with the broader update in the schema.ts file where JSONSchema is now imported as a type rather than a namespace.
49-49: Type import simplification from JSONSchema.JSONSchema to JSONSchemaThis change aligns with the broader update in the schema.ts file where JSONSchema is now imported as a type rather than a namespace.
packages/openapi/src/openapi-input-structure-parser.ts (3)
13-13: Type import simplification from JSONSchema.JSONSchema to JSONSchemaThis change aligns with the broader update in the schema.ts file where JSONSchema is now imported as a type rather than a namespace.
24-24: Schema converter call pattern updated for consistencyThe schema conversion call has been simplified to use array destructuring directly. This pattern is more consistent with the changes in openapi-output-structure-parser.ts and improves code readability.
45-45: Method signature types updated from JSONSchema.JSONSchema to JSONSchemaThese method signature changes align with the broader update in the schema.ts file where JSONSchema is now imported as a type rather than a namespace.
Also applies to: 86-86
packages/openapi/src/openapi-output-structure-parser.ts (3)
9-9: Type import simplification from JSONSchema.JSONSchema to JSONSchemaThis change aligns with the broader update in the schema.ts file where JSONSchema is now imported as a type rather than a namespace.
19-19: Schema converter call pattern updated for consistencyThe schema conversion call has been simplified to use array destructuring directly. This pattern is more consistent with the changes in openapi-input-structure-parser.ts and improves code readability.
37-37: Method signature types updated from JSONSchema.JSONSchema to JSONSchemaThese method signature changes align with the broader update in the schema.ts file where JSONSchema is now imported as a type rather than a namespace.
Also applies to: 60-60
packages/openapi/src/schema.ts (4)
1-3: Import structure improved for better type handlingThe changes to the import structure are beneficial:
- Added an eslint-disable for restricted imports to handle direct import from json-schema-typed
- Changed from importing JSONSchema as a namespace to importing specific types and named exports
- This allows for more precise type imports throughout the codebase
5-6: Export structure aligned with new import patternThe exports have been updated to match the new import pattern, maintaining consistency throughout the codebase.
8-9: Type definitions simplified to use direct JSONSchema referenceThe type definitions for ObjectSchema and FileSchema now use JSONSchema directly instead of JSONSchema.JSONSchema, which is more concise and aligns with the new import structure.
40-40: Satisfies clause updated to reference new JSONSchemaKeywords importThe satisfies clause has been updated to reference the newly imported JSONSchemaKeywords instead of JSONSchema.keywords, maintaining type checking while aligning with the new import structure.
packages/openapi/src/schema-converter.ts (5)
4-4: Clear separation of strategies.
This new type definition succinctly distinguishes between input and output schemas. Great addition!
15-15: Correct use of typed converters array.
UsingConditionalSchemaConverter[]accurately reflects the interface requirement for conditional checks.
17-17: Constructor injection is neat and testable.
This design supports flexible composition of converters.
21-21: Tuple return type is consistent.
Returning[required, jsonSchema]is a good approach and aligns with the updated interface.
23-24: Well-organized conditional flow.
Checkingcondition(schema, strategy)before delegating the conversion is clear and maintainable.packages/openapi/src/schema-utils.ts (7)
5-5: Consistent type usage for file schema.
Switching fromJSONSchema.JSONSchematoJSONSchemasimplifies and standardizes the interface.
9-9: Concise object schema check.
The updated type reference looks good; no concerns here.
13-13: Straightforward “any” schema validation.
This change maintains clarity.
17-17: Undefinable schema logic remains intact.
Retaining the same logic with simpler types is clear and consistent.
39-39: Typed “matched” properties reduce.
UsingRecord<string, JSONSchema>with the reduce function is appropriately typed.
61-61: Typed “rest” properties reduce.
Maintains the same pattern for the remaining properties with proper type annotations.
83-86: Consistent parameter types in filterSchemaBranches.
These changes correctly unify the schema type usage across the utility methods.eslint.config.js (1)
22-27: Good addition of the restricted imports rule.The ESLint rule to restrict imports from 'json-schema-typed' is well-structured and provides clear guidance to developers about using '@orpc/openapi' instead. This aligns with the PR's goal of rewriting the @orpc/zod component.
packages/zod/src/index.ts (3)
1-5: Well-organized imports for new schema types.The imports are cleanly organized and provide a clear indication of the modular structure being implemented.
9-14: Good modular export structure.The exports have been restructured to support a more modular approach, making individual schema types directly accessible. This approach enhances maintainability and allows for more granular imports by consumers.
16-22: Excellent aggregation of schemas with the 'oz' object.The 'oz' object provides a convenient single access point for all schema types, which simplifies usage while still maintaining the benefits of the modular structure. This is a good pattern for library design.
packages/zod/src/custom-json-schema.ts (3)
4-6: Good use of symbols for schema identification.Using symbols for storing custom JSON schemas prevents naming collisions and provides clear separation between different types of schemas (input, output, both).
8-25: Well-implemented schema retrieval function.The
getCustomJsonSchemafunction effectively retrieves custom JSON schemas based on the requested strategy, with proper type safety and clear control flow.
27-57: Type-safe implementation of custom JSON schema creation.The
customJsonSchemafunction is well-designed with:
- Proper generic typing that preserves the original schema type
- Conditional type logic for the strategy parameter
- Clean implementation for creating a new schema with the custom JSON schema attached
This implementation provides a flexible and type-safe way to enhance Zod schemas with custom JSON schema information.
packages/zod/src/coercer.test.ts (6)
6-15: Clear type and enum definitions for test cases.The test case type definition and enum are well-structured and provide a clear foundation for the comprehensive test suite.
17-143: Comprehensive test cases for native types.The native type test cases thoroughly cover various coercion scenarios including:
- Numbers and BigInt with different string representations
- Boolean values with various truthy/falsy string formats
- Date conversions from ISO strings
- Literal values and enum handling
This provides excellent coverage for the core coercion functionality.
145-236: Thorough testing of combination and wrapped schemas.The combination cases effectively test more complex scenarios:
- Union types
- Object intersections
- Various schema wrappers (.readonly(), .pipe(), etc.)
- Nested lazy schemas
This ensures the coercion logic works correctly with Zod's composition features.
238-259: Good coverage of custom schema types.The tests for URL and RegExp custom schemas ensure that the specialized coercion logic for these types works correctly, including both valid and invalid input handling.
274-360: Excellent test structure for thorough validation.The test structure using
describe.eachcombined with multiple sub-tests provides thorough validation of how coercion works in different contexts:
- Simple values
- Within objects, arrays, tuples, sets, and maps
- With missing properties
- With catchall properties and rest parameters
This systematic approach ensures comprehensive coverage of the coercion functionality.
362-393: Important test for non-Zod schema handling.Testing that the plugin correctly ignores non-Zod schemas is vital for ensuring compatibility when mixed schema systems are used. The test correctly verifies that inputs are passed through unchanged for non-Zod schemas.
packages/zod/src/converter.ts (1)
2-637: Robust Class-Based Zod → JSON Schema ConversionThe new
ZodToJsonSchemaConverterclass and itsconvertmethod provide excellent clarity over the previous procedural approach, with a good balance of configurability (e.g.,maxLazyDepth,anyJsonSchema,unsupportedJsonSchema). A few notes:
NaN →
type: 'null'
ForZodNaN, lines 263–267 map output to{ type: 'null' }. This might be slightly misleading if clients expect a numeric type. Confirm the design choice thatNaNis translated tonullfor the “output” scenario, ensuring downstream consumers anticipate that logic.Default Marking as Non-Required
InZodDefault(lines 530–536), returning[false, { default: schema_._def.defaultValue(), ...json }]means the property is optional. This is typically correct, as JSON Schemadefaultdoes not forcibly require a field. Just confirm that your consumers rely on the default for missing values as intended.Lazy Depth Handling
The fallback tothis.anyJsonSchemawith[false, this.anyJsonSchema](line 509–510) is a neat way to handle nested lazy schemas beyond themaxLazyDepth. Be sure to confirm nested references do not accidentally short-circuit deeper conversions if you intend to track them differently.Overall, the approach is thorough and extensible for future schema expansion.
| if (dynamic.length === 0) { | ||
| if (method === 'GET') { | ||
| let querySchema: JSONSchema.JSONSchema | undefined = inputSchema | ||
| let querySchema: JSONSchema | undefined = inputSchema |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Add test coverage for this line
This line appears to be flagged by code coverage tools as not covered by tests.
🏁 Script executed:
#!/bin/bash
# Check if there are any existing tests covering this code path
rg -A 5 -B 5 "parseCompactSchema.*querySchema.*method.*GET" --glob "**/*test*.ts" packages/openapi/Length of output: 101
Action Required: Add Test Coverage for Query Schema Assignment
It appears that the initialization of querySchema at line 91 in packages/openapi/src/openapi-input-structure-parser.ts is currently not exercised by any existing tests. A search for test references (using patterns like "parseCompactSchema.*querySchema.*method.*GET" and a more general search for "querySchema") returned no findings in our openapi-related test files.
- Location:
packages/openapi/src/openapi-input-structure-parser.tsat line 91let querySchema: JSONSchema | undefined = inputSchema
- Recommendation: Please add unit tests to cover this code path. The tests should verify:
- The scenario where
inputSchemacorrectly initializesquerySchema. - Any edge cases, such as handling when
inputSchemais undefined, if applicable.
- The scenario where
These changes are required to ensure full code coverage and to validate the intended behavior of the parser.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 91-91: packages/openapi/src/openapi-input-structure-parser.ts#L91
Added line #L91 was not covered by tests
1 participant
Closes: https://github.com/unnoq/orpc/issues/103
Summary by CodeRabbit
Chores
@types/nodeandzodacross multiple packages.New Features
Refactor
Tests