fix(schema): prioritize oneOf/anyOf/allOf over inferred type resolution#3781
Conversation
|
Could you resolve conflicts? |
c82482f to
6b9d420
Compare
|
Hi @kamilmysliwiec, conflicts resolved! Rebased onto latest master and all 187 tests pass. Ready for review. |
There was a problem hiding this comment.
Pull request overview
This PR fixes Swagger schema generation so explicitly-declared schema combinators (oneOf/anyOf/allOf) take precedence over inferred runtime types (notably Object from union types), preventing incorrect $ref: '#/components/schemas/Object' output (issue #3549).
Changes:
- Short-circuit schema type resolution in
SchemaObjectFactory.createSchemaMetadata()when a combinator is explicitly provided. - Add unit tests covering
oneOfwithObjectdesign:type, plugin metadata factory behavior, and broken lazy type function names. - Add e2e coverage demonstrating
oneOfwith$refto another schema (including plugin-metadata scenario) and update the generatedapi-spec.json.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
lib/services/schema-object-factory.ts |
Makes combinators take precedence by returning early before inferred-type resolution runs. |
test/services/schema-object-factory.spec.ts |
Adds regression tests for oneOf + Object and broken lazy-type inference scenarios. |
e2e/validate-schema.e2e-spec.ts |
Extends plugin metadata to cover the new oneOfWithRef field. |
e2e/src/cats/classes/cat.class.ts |
Adds oneOfWithRef DTO property using $ref combinator and registers TagDto as an extra model. |
e2e/api-spec.json |
Updates expected generated OpenAPI output to include oneOfWithRef. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Manually set the metadata as the plugin would, with a broken lazy type | ||
| Reflect.defineMetadata( | ||
| 'swagger/apiModelProperties', |
There was a problem hiding this comment.
Good call — switched to DECORATORS.API_MODEL_PROPERTIES to match the rest of the spec file and stay resilient to any internal constant rename.
|
@maruthang could you address copilot comments |
When a schema combinator (oneOf, anyOf, allOf) is explicitly declared via @ApiProperty, skip type-based resolution in createSchemaMetadata. This prevents the inferred type (e.g., Object from union types) from generating an incorrect $ref to '#/components/schemas/Object' instead of honoring the declared combinator. Closes nestjs#3549
6b9d420 to
cc48bab
Compare
|
Addressed both Copilot comments @kamilmysliwiec:
Also rebased onto latest |
|
LGTM |
|
@maruthang had to revert this PR as it broke unit tests on master |
PR Checklist
PR Type
What is the current behavior?
When using
@ApiProperty({ oneOf: [...] })on a property whose TypeScript type resolves toObjectat runtime (e.g., union types likeExample | Example[]), the generated swagger schema produces$ref: '#/components/schemas/Object'instead of the declaredoneOfcombinator.This happens because
createSchemaMetadata()resolves the inferred type before the combinator check inextractPropertiesFromType()has a chance to handle it. When the lazy type function name is not recognized (e.g., due to bundler/minifier transformations), the type falls through tocreateNotBuiltInTypeReference()which generates the incorrect$ref.Issue Number: #3549
What is the new behavior?
When a schema combinator (
oneOf,anyOf,allOf) is explicitly declared in the metadata,createSchemaMetadata()now returns early with the combinator intact, skipping type-based resolution entirely. This ensures the declared combinator always takes precedence over the inferred type.Does this PR introduce a breaking change?
Other information
oneOfusing$refto another schema (with and without plugin metadata)