Skip to content

Conversation

@AliRaZa1121
Copy link
Contributor

…rays); avoid lazy type for enum unions

Context:
When a DTO property used a union like Enum | null (or Enum[] | null) the plugin sometimes:

  • emitted a lazy type: () => FooEnum (unnecessary for enums; can cause circular refs), or
  • missed enum: FooEnum entirely for optional+nullable cases (e.g. ?: Status | null).

Root cause:

  • createTypePropertyAssignments didn’t special-case enum unions, so it fell back to a lazy type.
  • createEnumPropertyAssignment could return early when the checker reported a union containing null/undefined or when array unwrapping wasn’t detected up front, dropping the enum.

What changed:

  1. Handle enum unions in createTypePropertyAssignments:

    • For unions with exactly one non-nullish member: * If that member (or its array element) is an enum (including auto-generated enum unions), do not emit type. * Only add nullable: true when null is part of the union.
    • This avoids circular resolvers and lets enum metadata be provided solely by createEnumPropertyAssignment.
  2. Rewrite createEnumPropertyAssignment to be union- and optional-safe:

    • Unwrap null/`undefined` from unions before enum detection.
    • Treat array unwrapping as optional (don’t bail if not detected initially).
    • Resolve auto-generated enum unions (string literal unions) to a referenceable type.
    • Preserve and/or re-detect isArray after resolving enum-like types.
    • Skip enum members (e.g., FooEnum.ONE) which aren’t referenceable as enum types.

Results:

  • Enum | null{ enum: FooEnum, nullable: true }
  • Enum[] | null{ enum: FooEnum, isArray: true, nullable: true }
  • ?: Enum | null{ required: false, enum: FooEnum, nullable: true }
  • Non-enum unions (e.g., string | null) still emit type + nullable (as before).
  • undefined continues to affect required (based on ?), not nullable — unchanged.

Tests:

  • Fixture test/plugin/fixtures/nullable.dto.ts asserts correct transpiled metadata.
  • Previously failing spec “should support & understand nullable type unions” now passes.

Affects:

  • lib/plugin/visitors/model-class.visitor.ts
  • test/plugin/fixtures/nullable.dto.ts

Fixes: #3519

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

…rays); avoid lazy `type` for enum unions

Context:
When a DTO property used a union like `Enum | null` (or `Enum[] | null`) the plugin sometimes:
- emitted a lazy `type: () => FooEnum` (unnecessary for enums; can cause circular refs), or
- missed `enum: FooEnum` entirely for optional+nullable cases (e.g. `?: Status | null`).

Root cause:
- `createTypePropertyAssignments` didn’t special-case enum unions, so it fell back to a lazy `type`.
- `createEnumPropertyAssignment` could return early when the checker reported a union containing null/undefined or when array unwrapping wasn’t detected up front, dropping the `enum`.

What changed:
1) Handle enum unions in `createTypePropertyAssignments`:
   - For unions with exactly one non-nullish member:
     * If that member (or its array element) is an enum (including auto-generated enum unions), do **not** emit `type`.
     * Only add `nullable: true` when `null` is part of the union.
   - This avoids circular resolvers and lets enum metadata be provided solely by `createEnumPropertyAssignment`.

2) Rewrite `createEnumPropertyAssignment` to be union- and optional-safe:
   - Unwrap `null`/\`undefined\` from unions before enum detection.
   - Treat array unwrapping as optional (don’t bail if not detected initially).
   - Resolve auto-generated enum unions (string literal unions) to a referenceable type.
   - Preserve and/or re-detect `isArray` after resolving enum-like types.
   - Skip enum **members** (e.g., `FooEnum.ONE`) which aren’t referenceable as enum types.

Results:
- `Enum | null` → `{ enum: FooEnum, nullable: true }`
- `Enum[] | null` → `{ enum: FooEnum, isArray: true, nullable: true }`
- `?: Enum | null` → `{ required: false, enum: FooEnum, nullable: true }`
- Non-enum unions (e.g., `string | null`) still emit `type` + `nullable` (as before).
- `undefined` continues to affect `required` (based on `?`), not `nullable` — unchanged.

Tests:
- Fixture `test/plugin/fixtures/nullable.dto.ts` asserts correct transpiled metadata.
- Previously failing spec “should support & understand nullable type unions” now passes.

Affects:
- lib/plugin/visitors/model-class.visitor.ts
- test/plugin/fixtures/nullable.dto.ts

Fixes: nestjs#3519
@kamilmysliwiec
Copy link
Member

LGTM

@kamilmysliwiec kamilmysliwiec merged commit c1a062f into nestjs:master Oct 16, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

swagger plugin cause circular dependency when having a field of optional and nullable enum

2 participants