feat(openapi): gen spec with common schemas - components.schemas#638
feat(openapi): gen spec with common schemas - components.schemas#638
components.schemas#638Conversation
## Common Schemas
Define reusable schema components that can be referenced across your OpenAPI specification:
```ts
const UserSchema = z.object({
id: z.string(),
name: z.string(),
email: z.string().email(),
})
const PetSchema = z.object({
id: z.string().transform(id => Number(id)).pipe(z.number()),
})
const spec = await generator.generate(router, {
commonSchemas: {
User: {
schema: UserSchema,
},
InputPet: {
strategy: 'input',
schema: PetSchema,
},
OutputPet: {
strategy: 'output',
schema: PetSchema,
},
},
})
```
:::info
The `strategy` option determines which schema definition to use when input and output types differ (defaults to `input`). This is needed because we cannot use the same `$ref` for both input and output in this case.
:::
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update introduces support for reusable "common schemas" with explicit input/output strategies in the OpenAPI generator and Zod schema converters. Documentation and integration tests are added, and playgrounds are updated to register shared schemas. The Zod and Zod v4 converters gain options for reference depth and structure limits. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OpenAPIGenerator
participant SchemaConverter
participant OpenAPIDoc
User->>OpenAPIGenerator: generate({ commonSchemas })
OpenAPIGenerator->>OpenAPIGenerator: #resolveCommonSchemas(commonSchemas)
OpenAPIGenerator->>SchemaConverter: convert(schema, { strategy, ... })
SchemaConverter-->>OpenAPIGenerator: JSON Schema component(s)
OpenAPIGenerator->>OpenAPIDoc: Add components.schemas
OpenAPIGenerator->>OpenAPIDoc: Generate paths using $ref to commonSchemas
OpenAPIGenerator-->>User: OpenAPI spec with reusable components
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Summary of Changes
Hello @unnoq, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request adds support for defining and referencing common schemas in the OpenAPI specification generated from your router. This allows for better organization and reusability of schema definitions within the spec, particularly useful for complex or recursive types. It includes options to handle schemas with different input/output transformations and control when references are used based on nesting depth.
Highlights
- Schema Referencing (
$ref): The generator now uses$refpointers to reference the defined common schemas within the generated OpenAPI paths, reducing duplication and improving readability of the spec. - Input/Output Strategy: Adds a
strategyoption for common schemas to specify whether the input or output transformation of a Zod schema should be used for the component definition, addressing cases where schemas transform data differently. - Depth Control for References: Adds a
minStructureDepthForRefoption to the schema converter to control the minimum nesting depth required for a schema to be replaced by a$refto a common component.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configureGemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a significant enhancement by adding support for components.schemas in the OpenAPI generator. This allows for defining reusable schemas, improving the modularity and readability of the generated OpenAPI specifications. Key changes include:
- A new
commonSchemasoption inOpenAPIGeneratorGenerateOptionsto define these reusable components. - A
strategyoption withincommonSchemasto handle schemas that have different input and output representations (e.g., due to Zod transforms). - Updates to the Zod schema converters (for both Zod v3 and Zod v4) to recognize and use these common components via
$ref, respecting a newminStructureDepthForRefoption to control inlining vs. referencing. - Introduction of a
maxStructureDepthoption in Zod converters as a safeguard against excessive recursion during schema conversion.
The implementation appears robust, with careful consideration for how schemas are resolved and referenced. The tests are comprehensive, covering various scenarios including recursive schemas, different strategies, and varying structure depths for references. The documentation has also been updated clearly.
The feedback provided focuses on minor clarity improvements for complex conditional logic and a note on the schema comparison technique, aiming for enhanced long-term maintainability. Overall, this is a well-executed feature that will be very beneficial.
More templates
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/hey-api
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/react
@orpc/react-query
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-aws-lambda
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/standard-server-peer
@orpc/svelte-query
@orpc/tanstack-query
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
components.schemacomponents.schema)
components.schema)components.schemas)
components.schemas)components.schemas
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
playgrounds/next/src/app/api/[[...rest]]/route.ts (1)
28-36: SamecommonSchemasissues as in the Svelte-Kit playgroundThe mixed naming (
TokenSchema) and missingstrategyflags recur here. Apply the same normalisation/refactor as suggested for+server.tsto keep all playgrounds aligned.playgrounds/nest/src/reference/reference.service.ts (1)
30-38: Replicated naming / strategy mismatchThe Nest playground copies the same
commonSchemasblock; please propagate the earlier fixes here as well to avoid divergent component IDs across specs generated by different demos.packages/openapi/src/openapi-generator.ts (1)
171-174:JSON.stringifyequality caveat (same as prior feedback)Using
stringifyJSONto decide if input/output variants are identical is pragmatic but relies on key order stability.
If generation logic changes, semantically-equivalent schemas with different key order will be treated as different.(See previous review on identical pattern.)
packages/openapi/src/openapi-generator.test.ts (1)
978-993: Same self-reference issue forUser– preferz.lazyThe pattern
get parent() { return User.optional() }recreates a schema each access and may cause converter cache misses.
Apply thez.lazypattern here as well to keep object identity stable and reduce allocations.
🧹 Nitpick comments (17)
playgrounds/contract-first/src/main.ts (1)
30-38: Consistent schema key naming in commonSchemasThe key
TokenSchemaincludes theSchemasuffix while other keys (e.g.,Credential) do not. Consider using a uniform naming convention for component keys, for example renaming toToken:- TokenSchema: { schema: TokenSchema }, + Token: { schema: TokenSchema },playgrounds/nuxt/server/routes/api/[...].ts (1)
27-35: Consistent schema key naming in commonSchemasWithin
commonSchemas, the keyTokenSchemadiverges from the pattern used by other entries. To maintain uniform component naming:- TokenSchema: { schema: TokenSchema }, + Token: { schema: TokenSchema },playgrounds/tanstack-start/src/routes/api/$.ts (1)
30-38: Consistent schema key naming in commonSchemasThe
TokenSchemakey adds an extra suffix compared to other schema keys. Standardize component names by renaming it:- TokenSchema: { schema: TokenSchema }, + Token: { schema: TokenSchema },playgrounds/solid-start/src/routes/api/[...rest].ts (1)
29-37: Consistent schema key naming in commonSchemasThe
commonSchemasmap usesTokenSchemaas a key, which is inconsistent with other entries (e.g.,Credential). Rename for consistency:- TokenSchema: { schema: TokenSchema }, + Token: { schema: TokenSchema },playgrounds/astro/src/pages/api/[...rest].ts (1)
29-37: Consistent schema key naming in commonSchemasThe mapping uses
TokenSchemawhile other entries omit the suffix. To keep component names consistent, rename toToken:- TokenSchema: { schema: TokenSchema }, + Token: { schema: TokenSchema },packages/zod/src/zod4/converter.test.ts (1)
10-33: Preferz.lazyfor recursion – getter hack is brittleUsing a getter that references
Schemainside its own definition relies on the variable being initialised later and is easy to break with refactors.z.lazyis the idiomatic, safe way:-const Schema = z.object({ - id: z.string(), - name: z.string(), - get parents() { - return z.array(Schema).optional() - }, -}) +const Schema: z.ZodType<any> = z.lazy(() => + z.object({ + id: z.string(), + name: z.string(), + parents: z.array(Schema).optional(), + }), +)The test expectations remain unchanged, but this avoids TDZ hazards and improves readability.
apps/content/docs/openapi/openapi-specification.md (1)
126-139: Variable name in snippet doesn’t existEarlier in the doc the generator is instantiated as
openAPIGenerator, but the snippet usesgenerator.generate(...), which will confuse readers copying the code.-const spec = await generator.generate(router, { +const spec = await openAPIGenerator.generate(router, {Additionally, align naming with the playground fixes (
TokenvsTokenSchema) to stay consistent across docs and code.packages/zod/src/converter.ts (2)
78-85: Drop the redundant default assignment onmaxStructureDepth.
maxStructureDepthis initialised to100and immediately overwritten in the constructor (options.maxStructureDepth ?? 10).
Keeping both values adds noise and can mislead readers about the real default.-private readonly maxStructureDepth: Exclude<ZodToJsonSchemaOptions['maxStructureDepth'], undefined> = 100 +private readonly maxStructureDepth: Exclude<ZodToJsonSchemaOptions['maxStructureDepth'], undefined>
107-115: Avoid O(N) component look-ups on every recursion.
toArray(options.components)is walked for every nested property; on large, deeply-nested schemas this becomes a hot path.Consider pre-processing
componentsonce (e.g.new Map(schema → component)) outside the recursion to achieve O(1) look-ups.packages/zod/src/converter.components.test.ts (1)
8-10: Avoidanyin test fixtures where possible.Replacing the return type with
z.ZodOptional<...>(orReturnType<typeof z.array>etc.) preserves type-safety without affecting test readability.packages/zod/src/zod4/converter.ts (2)
88-90: Misleading default value formaxStructureDepthThe field is initialised to
100but overwritten in the constructor withoptions.maxStructureDepth ?? 10.
Drop the redundant initializer to avoid confusion.- private readonly maxStructureDepth: Exclude<experimental_ZodToJsonSchemaOptions['maxStructureDepth'], undefined> = 100 + private readonly maxStructureDepth: Exclude<experimental_ZodToJsonSchemaOptions['maxStructureDepth'], undefined>
130-138: Component lookup can become O(N²)
toArray(options.components)allocates a new array on every recursive call and linear-scans it.
For deeply-nested schemas with many components this degrades performance.Consider:
- caching
componentsoutside recursion- converting the array to a
Map<AnySchema, SchemaConverterComponent>for O(1) look-ups.packages/openapi/src/openapi-generator.ts (2)
161-189: Duplicate component entries risk
for … in commonSchemasblindlypushes intobaseOptions.components.
If a user supplies two aliases for the sameschemathe converter will store two identical components and$refresolution may become ambiguous.Deduplicate by schema (reference equality) or by
keybefore pushing.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 179-180: packages/openapi/src/openapi-generator.ts#L179-L180
Added lines #L179 - L180 were not covered by tests
194-205: Potential quadratic convert callsEach common schema is converted once per strategy inside the outer loop, then converted again when materialising
doc.components.schemas.
For large sets this doubles work; caching the earlierjsonwould avoid re-conversion.packages/openapi/src/schema-converter.ts (1)
5-10: Mark component fields immutable & clarify intent
SchemaConverterComponentinstances are treated as value-objects once registered.
Making their fieldsreadonlyprevents accidental mutation (e.g..refrewrites during conversion) and immediately signals immutability to readers.-export interface SchemaConverterComponent { - allowedStrategies: SchemaConvertOptions['strategy'][] - schema: AnySchema - required: boolean - ref: string -} +export interface SchemaConverterComponent { + readonly allowedStrategies: SchemaConvertOptions['strategy'][] + readonly schema: AnySchema + /** Whether the component itself is required when referenced */ + readonly required: boolean + /** Fully-qualified `$ref` pointing into `#/components/schemas/...` */ + readonly ref: string +}packages/zod/src/zod4/converter.components.test.ts (1)
192-210: ShadowingPetobscures earlier constantRe-declaring
const Petinside the"unsupported strategy"test block hides the outer-scopePet, which may confuse maintainers skimming the file.Rename the inner constant or inline the schema:
-const Pet = z.object({ +const UnsupportedPet = z.object({ id: z.string(), name: z.string(), })packages/openapi/src/openapi-generator.test.ts (1)
978-1393: Monster assertion block – extract helpers for readabilityThe single
"generator - commonSchemas"test spans ~400 lines of deeply nested expectations, which is hard to maintain. Consider:
- Splitting the test into smaller
itblocks (e.g. “components are emitted”, “/user route”, “/pet route”…).- Extracting expectation builders:
function expectRef(schemaName: string) { return { $ref: `#/components/schemas/${schemaName}` } }
- Using
expect.objectContaining+ helpers to reduce duplication.This refactor will make failures more actionable and future contributors less hesitant to touch the spec.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/content/docs/openapi/openapi-specification.md(1 hunks)packages/openapi/src/openapi-generator.test.ts(2 hunks)packages/openapi/src/openapi-generator.ts(10 hunks)packages/openapi/src/schema-converter.ts(1 hunks)packages/zod/src/converter.components.test.ts(1 hunks)packages/zod/src/converter.ts(17 hunks)packages/zod/src/zod4/converter.components.test.ts(1 hunks)packages/zod/src/zod4/converter.test.ts(1 hunks)packages/zod/src/zod4/converter.ts(19 hunks)playgrounds/astro/src/pages/api/[...rest].ts(2 hunks)playgrounds/contract-first/src/main.ts(2 hunks)playgrounds/nest/src/reference/reference.service.ts(2 hunks)playgrounds/next/src/app/api/[[...rest]]/route.ts(2 hunks)playgrounds/nuxt/server/routes/api/[...].ts(2 hunks)playgrounds/solid-start/src/routes/api/[...rest].ts(2 hunks)playgrounds/svelte-kit/src/routes/api/[...rest]/+server.ts(2 hunks)playgrounds/tanstack-start/src/routes/api/$.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (11)
playgrounds/tanstack-start/src/routes/api/$.ts (3)
playgrounds/tanstack-start/src/schemas/user.ts (2)
NewUserSchema(7-22)UserSchema(24-39)playgrounds/tanstack-start/src/schemas/auth.ts (2)
CredentialSchema(3-6)TokenSchema(8-10)playgrounds/tanstack-start/src/schemas/planet.ts (3)
NewPlanetSchema(9-13)UpdatePlanetSchema(15-20)PlanetSchema(22-28)
playgrounds/astro/src/pages/api/[...rest].ts (3)
playgrounds/astro/src/schemas/user.ts (2)
NewUserSchema(7-22)UserSchema(24-39)playgrounds/astro/src/schemas/auth.ts (2)
CredentialSchema(3-6)TokenSchema(8-10)playgrounds/astro/src/schemas/planet.ts (3)
NewPlanetSchema(9-13)UpdatePlanetSchema(15-20)PlanetSchema(22-28)
playgrounds/solid-start/src/routes/api/[...rest].ts (3)
playgrounds/solid-start/src/schemas/user.ts (2)
NewUserSchema(7-22)UserSchema(24-39)playgrounds/solid-start/src/schemas/auth.ts (2)
CredentialSchema(3-6)TokenSchema(8-10)playgrounds/solid-start/src/schemas/planet.ts (3)
NewPlanetSchema(9-13)UpdatePlanetSchema(15-20)PlanetSchema(22-28)
playgrounds/svelte-kit/src/routes/api/[...rest]/+server.ts (3)
playgrounds/svelte-kit/src/schemas/user.ts (2)
NewUserSchema(7-22)UserSchema(24-39)playgrounds/svelte-kit/src/schemas/auth.ts (2)
CredentialSchema(3-6)TokenSchema(8-10)playgrounds/svelte-kit/src/schemas/planet.ts (3)
NewPlanetSchema(9-13)UpdatePlanetSchema(15-20)PlanetSchema(22-28)
playgrounds/nest/src/reference/reference.service.ts (3)
playgrounds/nest/src/schemas/user.ts (2)
NewUserSchema(7-22)UserSchema(24-39)playgrounds/nest/src/schemas/auth.ts (2)
CredentialSchema(3-6)TokenSchema(8-10)playgrounds/nest/src/schemas/planet.ts (3)
NewPlanetSchema(9-13)UpdatePlanetSchema(15-20)PlanetSchema(22-28)
packages/zod/src/zod4/converter.test.ts (2)
packages/zod/src/zod4/converter.ts (1)
required(577-592)packages/zod/src/converter.ts (1)
required(663-671)
packages/zod/src/converter.components.test.ts (2)
packages/zod/src/converter.ts (2)
ZodToJsonSchemaConverter(76-672)required(663-671)packages/zod/src/zod4/converter.ts (1)
required(577-592)
packages/openapi/src/schema-converter.ts (1)
packages/contract/src/schema.ts (1)
AnySchema(7-7)
packages/zod/src/converter.ts (4)
packages/openapi/src/schema.ts (1)
JSONSchema(6-6)packages/openapi/src/schema-converter.ts (1)
ConditionalSchemaConverter(34-36)packages/zod/src/zod4/converter.ts (3)
required(577-592)schema(115-552)schema(554-575)packages/shared/src/array.ts (1)
toArray(1-3)
packages/zod/src/zod4/converter.components.test.ts (1)
packages/zod/src/zod4/converter.ts (1)
required(577-592)
playgrounds/next/src/app/api/[[...rest]]/route.ts (3)
playgrounds/next/src/schemas/user.ts (2)
NewUserSchema(7-22)UserSchema(24-39)playgrounds/next/src/schemas/auth.ts (2)
CredentialSchema(3-6)TokenSchema(8-10)playgrounds/next/src/schemas/planet.ts (3)
NewPlanetSchema(9-13)UpdatePlanetSchema(15-20)PlanetSchema(22-28)
🪛 GitHub Check: codecov/patch
packages/zod/src/converter.ts
[warning] 104-105: packages/zod/src/converter.ts#L104-L105
Added lines #L104 - L105 were not covered by tests
packages/openapi/src/openapi-generator.ts
[warning] 179-180: packages/openapi/src/openapi-generator.ts#L179-L180
Added lines #L179 - L180 were not covered by tests
🔇 Additional comments (6)
playgrounds/contract-first/src/main.ts (1)
9-11: Imports common schemas for OpenAPI specThe new imports correctly bring in the shared Zod schemas for users, auth, and planets, which will be registered as reusable components.
playgrounds/nuxt/server/routes/api/[...].ts (1)
6-8: Imports common schemas in Nuxt API routeThe updated imports correctly load the shared Zod schemas from the server directory for inclusion in the OpenAPI spec.
playgrounds/tanstack-start/src/routes/api/$.ts (1)
9-11: Imports shared schemas for TanStack API routeThese imports correctly reference the common Zod schemas for User, Auth, and Planet, enabling their reuse across the OpenAPI spec.
playgrounds/solid-start/src/routes/api/[...rest].ts (1)
8-10: Import reusable schemas for Solid StartThe new imports correctly include the common Zod schemas, ensuring they're available for component registration in the OpenAPI spec.
playgrounds/astro/src/pages/api/[...rest].ts (1)
8-10: Import common Zod schemas in Astro routeThe added imports correctly fetch the shared schemas from the Astro project, ready for OpenAPI component registration.
packages/zod/src/converter.ts (1)
103-105: No tests exercise the max-depth fallback path.The branch returning
anyJsonSchemawhenstructureDepth > this.maxStructureDepthis currently uncovered.
Please add a unit test that creates a schema nested beyond the limit to keep this safeguard from regressing.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 104-105: packages/zod/src/converter.ts#L104-L105
Added lines #L104 - L105 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/zod/src/converter.test.ts (2)
630-637: Guard against excessive recursion depth
createSchema(100)builds a 100-level nested object through synchronous recursion.
While fine in CI, increasing depth (or user-supplied input) could exceed the JS call-stack.
Consider:
- Using
z.lazywithmaxLazyDepthto cap traversal.- Or refactoring
createSchemato an iterative build if deeper trees might be tested later.Not blocking, but worth keeping in mind for long-running fuzz tests.
641-655: Add stronger assertions on nested “children” objectsCurrently the expectation only checks for
items: { type: 'object' }, which would pass even if
required keys (id,name) were omitted in deeper levels.- children: { - type: 'array', - items: expect.objectContaining({ type: 'object' }), - }, + children: { + type: 'array', + items: expect.objectContaining({ + type: 'object', + required: ['id', 'name'], + }), + },This tiny addition fortifies the test against regressions that strip the inner-object structure while keeping the outer signature intact.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/openapi/src/openapi-generator.test.ts(2 hunks)packages/zod/src/converter.test.ts(1 hunks)packages/zod/src/converter.ts(16 hunks)packages/zod/src/zod4/converter.ts(18 hunks)playgrounds/astro/src/pages/api/[...rest].ts(2 hunks)playgrounds/contract-first/src/main.ts(2 hunks)playgrounds/nest/src/reference/reference.service.ts(2 hunks)playgrounds/next/src/app/api/[[...rest]]/route.ts(2 hunks)playgrounds/nuxt/server/routes/api/[...].ts(2 hunks)playgrounds/solid-start/src/routes/api/[...rest].ts(2 hunks)playgrounds/svelte-kit/src/routes/api/[...rest]/+server.ts(2 hunks)playgrounds/tanstack-start/src/routes/api/$.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- playgrounds/astro/src/pages/api/[...rest].ts
🚧 Files skipped from review as they are similar to previous changes (10)
- playgrounds/contract-first/src/main.ts
- playgrounds/tanstack-start/src/routes/api/$.ts
- playgrounds/next/src/app/api/[[...rest]]/route.ts
- playgrounds/svelte-kit/src/routes/api/[...rest]/+server.ts
- playgrounds/solid-start/src/routes/api/[...rest].ts
- playgrounds/nuxt/server/routes/api/[...].ts
- playgrounds/nest/src/reference/reference.service.ts
- packages/openapi/src/openapi-generator.test.ts
- packages/zod/src/converter.ts
- packages/zod/src/zod4/converter.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint
- GitHub Check: publish-commit
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/openapi/src/openapi-generator.ts (1)
171-179:stringifyJSONequality caveat already notedThe equality check that relies on
stringifyJSONsuffers from key-ordering issues (see earlier review on lines 171/178 of this file).
If the order changes the comparison returns false even though the schemas are semantically identical.
A deep structural comparison would be safer.
🧹 Nitpick comments (4)
packages/openapi/src/openapi-utils.ts (1)
168-179: Consider supporting JSON-Pointer escaping to avoid ambiguous keys
resolveOpenAPIJsonSchemaRefextracts the slice after#/components/schemas/verbatim ('c/c'stays'c/c').
According to the JSON-Pointer spec, slashes within a key must be escaped as~1; consumers that rely on strict pointers will therefore fail to dereference#/components/schemas/c/c.You could transparently unescape
~1→/(and~0→~) when looking up the component, and, more importantly, escape component names when you generate$refs.-const name = schema.$ref.slice(OPENAPI_JSON_SCHEMA_REF_PREFIX.length) +const raw = schema.$ref.slice(OPENAPI_JSON_SCHEMA_REF_PREFIX.length) +const name = raw.replace(/~1/g, '/').replace(/~0/g, '~')Doing so keeps you RFC-6901-compliant and prevents subtle interop issues.
packages/openapi/src/openapi-utils.test.ts (2)
333-348: Tests rely on non-spec$ref– encode/with~1The case that checks
#/components/schemas/c/cpasses only because the implementation treats the whole tail as a property name.
Strict JSON-Pointer rules require the slash to be escaped (#/components/schemas/c~1c). Otherwise a compliant tool would look forcomponents.schemas.c.c.Updating the fixture keeps the test future-proof once pointer-escaping is supported:
- 'c/c': { type: 'object' }, + 'c/c': { type: 'object' }, ... - expect(resolveOpenAPIJsonSchemaRef(doc, { $ref: '#/components/schemas/c/c' })).toEqual({ type: 'object' }) + expect(resolveOpenAPIJsonSchemaRef(doc, { $ref: '#/components/schemas/c~1c' })).toEqual({ type: 'object' })
357-365: Minor phrasing / grammar
it('it do nothing if have no components.schemas' …)reads awkwardly.
Considerit('does nothing when components.schemas is missing', …)for clarity.packages/openapi/src/openapi-generator.ts (1)
88-90: Prefer deletingcommonSchemasover assigningundefined
doc.commonSchemas = undefinedleaves an own property on the object that serialises tonullin JSON.
Remove the key instead so it never leaks into the final spec.- commonSchemas: undefined, + // strip generator-specific option so it never appears in the output + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + delete (doc as any).commonSchemas,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/openapi/src/openapi-generator.test.ts(2 hunks)packages/openapi/src/openapi-generator.ts(15 hunks)packages/openapi/src/openapi-utils.test.ts(2 hunks)packages/openapi/src/openapi-utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/openapi/src/openapi-generator.test.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: publish-commit
- GitHub Check: lint
Common Schemas
Define reusable schema components that can be referenced across your OpenAPI specification:
The
strategyoption determines which schema definition to use when input and output types differ (defaults toinput). This is needed because we cannot use the same$reffor both input and output in this case.Summary by CodeRabbit
New Features
Documentation
Tests
$refreferences within OpenAPI documents.Chores