-
-
Notifications
You must be signed in to change notification settings - Fork 121
feat(openapi): add TypeDef support to RPC OpenAPI generator #2222
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
- Add TypeDef (custom type) generation to RPC OpenAPI spec - Generate OpenAPI schemas for all TypeDefs defined in zmodel - Map Json fields with @JSON decorator to reference TypeDef schemas - Support complex TypeDef structures including: - Nested TypeDefs (TypeDefs referencing other TypeDefs) - Arrays of TypeDefs - Enums in TypeDefs - Optional fields - Handle enums that are only used in TypeDefs (not in models) - Add comprehensive tests for TypeDef support - Update baseline files with new TypeDef fields 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds AST Enum and TypeDef support to the OpenAPI RPC generator: emits components for Enums and TypeDefs, maps TypeDef fields (including Json fields referencing TypeDefs) to OpenAPI schemas, updates field-generation signatures, and adds tests for nested TypeDef structures and Json handling. Changes
Sequence Diagram(s)sequenceDiagram
participant AST as AST (Enum/TypeDef)
participant DMMF as Prisma DMMF
participant Gen as RPCOpenAPIGenerator
participant OAPI as OpenAPI Components
Gen->>DMMF: collect models & fields
Gen->>AST: collect Enum & TypeDef declarations
Gen->>OAPI: emit Enum schemas (if absent)
Gen->>OAPI: emit TypeDef schemas via generateTypeDefComponent
loop for each model.field
Gen->>Gen: generateField(field, modelName?)
alt field is Json referencing TypeDef
Gen->>OAPI: reference TypeDef schema (#/components/schemas/...)
else
Gen->>OAPI: emit scalar/model/enum schema or ref
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 2
🔭 Outside diff range comments (1)
packages/plugins/openapi/src/rpc-generator.ts (1)
902-922
: Remove ineffective TypeDef check in prismaTypeToOpenAPIType; simplify Json handling
isTypeDefType
can never be true under the'JSON' | 'Json'
branch becausetype
is literally'JSON'|'Json'
, not a TypeDef name. This makes the branch misleading dead logic. Recommend simplifying to return{}
for Json here and keep TypeDef-aware Json mapping ingenerateField
, where you have AST context.Apply this diff to clean up:
private prismaTypeToOpenAPIType(type: string, nullable: boolean): OAPI.ReferenceObject | OAPI.SchemaObject { - // Check if this type is a TypeDef - const isTypeDefType = this.model.declarations.some(d => isTypeDef(d) && d.name === type); - const result = match(type) .with('String', () => ({ type: 'string' })) .with(P.union('Int', 'BigInt'), () => ({ type: 'integer' })) .with('Float', () => ({ type: 'number' })) .with('Decimal', () => this.oneOf({ type: 'string' }, { type: 'number' })) .with(P.union('Boolean', 'True'), () => ({ type: 'boolean' })) .with('DateTime', () => ({ type: 'string', format: 'date-time' })) .with('Bytes', () => ({ type: 'string', format: 'byte' })) - .with(P.union('JSON', 'Json'), () => { - // For Json fields, check if there's a specific TypeDef reference - // Otherwise, return empty schema for arbitrary JSON - return isTypeDefType ? this.ref(type, false) : {}; - }) + .with(P.union('JSON', 'Json'), () => ({})) .otherwise((type) => this.ref(type.toString(), false)); return this.wrapNullable(result, nullable); }
🧹 Nitpick comments (2)
packages/plugins/openapi/src/rpc-generator.ts (1)
761-776
: Json→TypeDef mapping via AST is correct; consider minor robustness tweakLogic is sound: for scalar Json fields, resolve the original DataModel field and, if it references a TypeDef via @JSON, emit a $ref. Two small polish items:
- Handle the unlikely but harmless alias "JSON" in addition to "Json".
- Optional: avoid repeated DataModel lookup by caching a map of modelName → DataModel (micro-optimization).
Apply this diff to allow "JSON" alias:
- if (def.kind === 'scalar' && def.type === 'Json' && modelName && def.name) { + if (def.kind === 'scalar' && (def.type === 'Json' || def.type === 'JSON') && modelName && def.name) {packages/plugins/openapi/tests/openapi-rpc.test.ts (1)
380-383
: Baselines not found — please update type-coverage baselines for new Meta TypeDef and Foo.json mappingI ran the provided verification script; no rpc-type-coverage baseline files were found and packages/plugins/openapi/tests/baseline does not exist, so I couldn't confirm the baselines include the new Meta TypeDef or Foo.json -> #/components/schemas/Meta mapping.
Please update the appropriate baselines (both 3.0.0 and 3.1.0) to include the new entries, e.g.:
type Meta { something String }
Targets to verify/update:
- rpc-type-coverage-*.baseline.yaml for 3.0.0
- rpc-type-coverage-*.baseline.yaml for 3.1.0
- Ensure each baseline contains:
- a Meta: type entry
- Foo: json: $ref: #/components/schemas/Meta
If you’d like, I can help regenerate the baselines once those files are present or tell me where they live.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
packages/plugins/openapi/tests/baseline/rpc-type-coverage-3.0.0.baseline.yaml
is excluded by!**/*.yaml
packages/plugins/openapi/tests/baseline/rpc-type-coverage-3.1.0.baseline.yaml
is excluded by!**/*.yaml
📒 Files selected for processing (2)
packages/plugins/openapi/src/rpc-generator.ts
(7 hunks)packages/plugins/openapi/tests/openapi-rpc.test.ts
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/plugins/openapi/tests/openapi-rpc.test.ts (3)
packages/testtools/src/schema.ts (2)
loadZModelAndDmmf
(388-420)normalizePath
(73-75)packages/plugins/openapi/src/rpc-generator.ts (1)
generate
(46-106)packages/plugins/openapi/src/rest-generator.ts (1)
generate
(59-107)
packages/plugins/openapi/src/rpc-generator.ts (2)
packages/runtime/src/local-helpers/upper-case-first.ts (1)
upperCaseFirst
(1-3)packages/language/src/ast.ts (1)
DataModel
(56-66)
🔇 Additional comments (5)
packages/plugins/openapi/src/rpc-generator.ts (4)
4-4
: AST imports for TypeDef/Enum support look correctBrings in the exact AST constructs used below for TypeDef/Enum generation and mapping. No concerns.
645-654
: Good: include AST-only enums to cover cases not present in DMMFThis ensures enums referenced only from TypeDefs (and thus absent from DMMF) still get emitted. Solid move.
661-665
: Good: emit TypeDef componentsAdding TypeDef components directly from AST closes the parity gap with the REST generator. Looks good.
748-748
: Passing the model name into generateField is necessaryThis is key for mapping Json fields back to AST for TypeDef references. Change is correct.
packages/plugins/openapi/tests/openapi-rpc.test.ts (1)
395-397
: LGTM: field type coverage includes a Json-typed field referencing MetaThis exercises the @JSON TypeDef reference mapping and keeps a plain Json for comparison. Good coverage.
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.
Hi @olup , thanks for making this PR! Overall looks great to me. Could you check my comments?
- Move isTypeDefType variable inside match.with block for Json type - Reuse prismaTypeToOpenAPIType method to avoid duplication - Change Json from { type: 'string', format: 'json' } to empty schema {} - Update test assertion for nullable metadata field These changes improve code quality by reducing duplication and aligning with OpenAPI specification best practices for JSON schema representation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
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
♻️ Duplicate comments (1)
packages/plugins/openapi/src/rpc-generator.ts (1)
867-881
: Scalar/ref mapping for TypeDef fields is sound; Json as arbitrary schema is consistent
- Referenced types (TypeDef/Enum/Model) resolve to component refs and get tracked for pruning.
- Json mapped to {} maintains consistency with the rest of the generator. This also addresses a prior concern about using a non-standard "json" string format.
🧹 Nitpick comments (3)
packages/plugins/openapi/src/rpc-generator.ts (3)
761-776
: Json→TypeDef mapping implemented; consider caching for minor perf gainsThe AST back-mapping for Json fields is correct and covers lists/optional. Minor optimization: repeated lookup of the same DataModel per field can be cached to avoid O(n_fields × n_models) scans.
Apply this diff locally and add a small cache helper:
- private generateField(def: { kind: DMMF.FieldKind; type: string; isList: boolean; isRequired: boolean; name?: string }, modelName?: string) { + private generateField( + def: { kind: DMMF.FieldKind; type: string; isList: boolean; isRequired: boolean; name?: string }, + modelName?: string + ) { // For Json fields, check if there's a corresponding TypeDef in the original model if (def.kind === 'scalar' && def.type === 'Json' && modelName && def.name) { - const dataModel = this.model.declarations.find(d => isDataModel(d) && d.name === modelName) as DataModel; + const dataModel = this.getZModelByName(modelName); if (dataModel) { const field = dataModel.fields.find(f => f.name === def.name); if (field?.type.reference?.ref && isTypeDef(field.type.reference.ref)) { // This Json field references a TypeDef return this.wrapArray( this.wrapNullable(this.ref(field.type.reference.ref.name, true), !def.isRequired), def.isList ); } } }Add this helper (outside the changed hunk) to the class:
// simple per-instance cache to avoid repeated scans private __zModelCache?: Map<string, DataModel>; private getZModelByName(name: string): DataModel | undefined { if (!this.__zModelCache) { this.__zModelCache = new Map( this.model.declarations.filter(isDataModel).map((m) => [m.name, m] as const) ); } return this.__zModelCache.get(name); }
842-858
: TypeDef component generation is correct; optionally restrict unknown fieldsThe properties/required derivation is right. Consider setting additionalProperties: false to better convey the closed shape of TypeDefs (unless your TypeDefs are intentionally open).
private generateTypeDefComponent(typeDef: TypeDef): OAPI.SchemaObject { const schema: OAPI.SchemaObject = { type: 'object', description: `The "${typeDef.name}" TypeDef`, properties: typeDef.fields.reduce((acc, field) => { acc[field.name] = this.generateTypeDefField(field); return acc; }, {} as Record<string, OAPI.ReferenceObject | OAPI.SchemaObject>), + additionalProperties: false, };
906-911
: Remove ineffective TypeDef check in Json mapping; always return unconstrained JSON hereIn this branch, type is literally 'JSON' or 'Json', so the isTypeDefType test will always be false. The TypeDef-aware Json mapping is already handled in generateField via AST back-mapping. Simplify to an unconstrained schema and drop the dead code/comment.
Apply this diff:
- .with(P.union('JSON', 'Json'), () => { - // For Json fields, check if there's a specific TypeDef reference - // Otherwise, return empty schema for arbitrary JSON - const isTypeDefType = this.model.declarations.some(d => isTypeDef(d) && d.name === type); - return isTypeDefType ? this.ref(type, false) : {}; - }) + .with(P.union('JSON', 'Json'), () => ({}))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/plugins/openapi/src/rpc-generator.ts
(6 hunks)packages/plugins/openapi/tests/openapi-rpc.test.ts
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/plugins/openapi/tests/openapi-rpc.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/plugins/openapi/src/rpc-generator.ts (2)
packages/runtime/src/local-helpers/upper-case-first.ts (1)
upperCaseFirst
(1-3)packages/language/src/ast.ts (1)
DataModel
(56-66)
🔇 Additional comments (5)
packages/plugins/openapi/src/rpc-generator.ts (5)
4-4
: Imports extended for Enum/TypeDef AST support — looks goodBrings in the necessary AST symbols and type guards to enable TypeDef/Enum handling. No issues spotted.
646-654
: AST-only enums inclusion is correct and avoids duplicationFilling gaps where enums are only used inside TypeDefs and not surfaced in DMMF. The existence check keyed by upper-cased name prevents duplicates. Good addition.
661-665
: Generating TypeDef components from AST — aligned with PR goalsThis ensures TypeDefs appear under components.schemas even when not present in DMMF. Together with reference tracking, this enables correct pruning behavior only for unused ones.
748-748
: Passing model name into field generation enables TypeDef-aware Json mappingThis change is necessary to map DMMF fields back to AST for discovering @JSON TypeDef references. Solid wiring.
860-865
: Array and nullability wrapping for TypeDef fields — correctCorrectly reflects optional and array semantics for TypeDef fields.
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
🔭 Outside diff range comments (1)
packages/plugins/openapi/src/rpc-generator.ts (1)
119-119
: Typo in warning message (extra brace at end)There’s an extra closing brace in the warning string.
- this.warnings.push(`Unable to load ZModel definition for: ${model.name}}`); + this.warnings.push(`Unable to load ZModel definition for: ${model.name}`);
🧹 Nitpick comments (6)
packages/plugins/openapi/src/rpc-generator.ts (6)
652-661
: Good: include AST-only enums to componentsCovers enums only used inside TypeDefs. Minor optional improvement: consider reusing a single enum-generation path (helper) for consistency between DMMF and AST sources.
768-783
: Nice: Json model fields mapped back to TypeDefs via ASTThis correctly upgrades Json fields annotated with @JSON into $ref TypeDef schemas. One small suggestion: guard against missing
def.name
even though DMMF fields should always have it.- if (def.kind === 'scalar' && def.type === 'Json' && modelName && def.name) { + if (def.kind === 'scalar' && def.type === 'Json' && modelName && def.name) { const dataModel = this.model.declarations.find(d => isDataModel(d) && d.name === modelName) as DataModel; if (dataModel) { const field = dataModel.fields.find(f => f.name === def.name); if (field?.type.reference?.ref && isTypeDef(field.type.reference.ref)) { // This Json field references a TypeDef return this.wrapArray( this.wrapNullable(this.ref(field.type.reference.ref.name, true), !def.isRequired), def.isList ); } } }
849-865
: Close TypeDef schemas with additionalProperties: false (optional)To make generated TypeDef objects stricter (and match typical OpenAPI expectations), consider disallowing extra properties.
private generateTypeDefComponent(typeDef: TypeDef): OAPI.SchemaObject { const schema: OAPI.SchemaObject = { type: 'object', description: `The "${typeDef.name}" TypeDef`, properties: typeDef.fields.reduce((acc, field) => { acc[field.name] = this.generateTypeDefField(field); return acc; }, {} as Record<string, OAPI.ReferenceObject | OAPI.SchemaObject>), + additionalProperties: false, };
365-369
: Use strict equality consistently in security inferenceMixing
==
with===
is inconsistent and unnecessary here.- security: create === true && update == true ? [] : undefined, + security: create === true && update === true ? [] : undefined,
836-842
: Remove stray no-op expression
field.outputType;
is a no-op and should be removed.- field.outputType; properties[field.name] = this.wrapArray(outputType, field.outputType.isList);
905-913
: Optional: improve BigInt hinting with int64 formatClients often expect BigInt as int64.
- .with(P.union('Int', 'BigInt'), () => ({ type: 'integer' })) + .with('Int', () => ({ type: 'integer', format: 'int32' })) + .with('BigInt', () => ({ type: 'integer', format: 'int64' }))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
packages/plugins/openapi/tests/baseline/rpc-type-coverage-3.0.0.baseline.yaml
is excluded by!**/*.yaml
packages/plugins/openapi/tests/baseline/rpc-type-coverage-3.1.0.baseline.yaml
is excluded by!**/*.yaml
📒 Files selected for processing (1)
packages/plugins/openapi/src/rpc-generator.ts
(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: OSSAR-Scan
🔇 Additional comments (6)
packages/plugins/openapi/src/rpc-generator.ts (6)
4-4
: AST imports expansion looks correctBringing in Enum/TypeDef symbols from the AST is appropriate for the new functionality.
668-671
: Good: TypeDefs are emitted as componentsThis is the right place to surface TypeDefs into OpenAPI.
755-755
: Passing modelName into generateField is correctThis enables AST lookups to resolve TypeDef references for Json model fields.
874-888
: TypeDef scalar mapping: good reuse of prismaTypeToOpenAPIType; JSON branch handled as arbitrary JSONThis keeps the mapping consistent with the rest of the generator. Looks good.
913-919
: JSON branch uses an ineffective type check; simplify to always return arbitrary JSON
type
here is 'JSON'/'Json', soisTypeDef
detection will never succeed. The earlier generateField covers Json->TypeDef mapping with model context already.
[ suggest_essential_refactor ]- .with(P.union('JSON', 'Json'), () => { - // For Json fields, check if there's a specific TypeDef reference - // Otherwise, return empty schema for arbitrary JSON - const isTypeDefType = this.model.declarations.some(d => isTypeDef(d) && d.name === type); - return isTypeDefType ? this.ref(type, false) : {}; - }) + .with(P.union('JSON', 'Json'), () => ({}))
668-671
: Ensure TypeDefs not referenced by any path are not pruned (verify)If pruneComponents only keeps components reachable from $refs used in paths, standalone TypeDefs (declared but unused) might be pruned. If the intention is to always emit all TypeDefs, consider marking them as roots or teaching prune to keep them.
Would you confirm if pruneComponents preserves TypeDefs that are not referenced by any schema? If not, I can propose a small change to seed them as roots or whitelist them during pruning.
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.
LGTM now. I've merged latest code from dev, resolved some conflicts. Will merge the PR once CI passes.
Thanks for the conflict resolution, I'll make sure to pull dev next time. |
Summary
This PR adds TypeDef (custom type) support to the RPC OpenAPI generator, bringing it to feature parity with the REST generator.
Changes
Technical Details
The RPC generator works with Prisma's DMMF which doesn't preserve TypeDef reference information. To solve this, the implementation:
Testing
Example
Given this zmodel:
The RPC OpenAPI generator now correctly generates schemas for Address, ContactInfo, and properly references them from the User model.
🤖 Generated with Claude Code