Skip to content
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

Fix colliding enumValues exports #2051

Merged
merged 7 commits into from
Jan 4, 2025

Conversation

duncanbeevers
Copy link
Contributor

@duncanbeevers duncanbeevers commented Dec 13, 2024

🗣️ Discussion

This work stemmed from an error encountered when using enumValues: true, and escalated from there.

🐞 The bugs

  • Types of exported literals are non-exhaustive.
  • Composing OpenAPI enums generates colliding variable names.

💨 Non-exhaustive types

The existing values exported when using enumValues express their type relative to the union defined in the TS schema.

// Before
export interface components {
  schemas: {
    /** @enum {string} */
    Status: "active" | "inactive";
  };
}
export const statusValues: ReadonlyArray<components["schemas"]["Status"]> = ["active", "inactive"];

Instead, if we start with an exhaustive concrete value, the union type can be derived.

// After
export const StatusValues = ["active", "inactive"] as const;
export type Status = (typeof StatusValues)[number];
export interface components {
  schemas: {
    Status: Status;
  }
}

➕ Composing OpenAPI enums generates conflicting variable names

Given the following schema composing two enum schemas, the following bad code is generated.

components:
  schemas:
    Status:
      anyOf:
        - type: string
          enum:
            - active
            - inactive
        - type: string
          enum:
            - appealed
export const statusValues: components["schemas"]["Status"] = ["active", "inactive"];
export const statusValues: components["schemas"]["Status"] = ["appealed"];

By unifying the enum member and literal string traversal + generation paths, I was able to generate non-conflicting export names.

Changes

  • Preserve capitalization of named, exported enum values.
  • Type exported const values as const, instead of their location within the operations or components schemas.
  • Derive and export types for enum values from concrete values in const arrays.
  • Use derived enum value types in operations and components schemas.
  • Use non-conflicting variable names for composed OpenAPI enums (anyOf: [enum1, enum2])
  • Export a type-predicate function for each enum constant.

How to Review

  • There's an unhealthy behavior around composition + enum: true. Since enums aren't composable, we still generate nonsense code like ModeratedStatus: components["schemas"]["Status"] | ModeratedStatusAnyOf1; (see the node-utils options > enum case for this example. This isn't resolved by this PR, and may be entirely unresolvable.
  • Helpers and consts and such are now prepended before the primary root exports; this is necessary since some schema are now derived from these const values.
  • Exported const values are no longer lower-cased. This eliminated some special-case code in a couple of spots, and allowed for exporting sanitizeMemberName and using it in transformers/schema-object. That alone makes this a breaking change.
  • The as const style literals may not be for everyone, though I've found them to be succinct, reliable, and versatile for composition (literal and type-level) and iteration.

Checklist

  • Unit tests updated
  • docs/ updated (if necessary)
  • pnpm run update:examples run (only applicable for openapi-typescript)

@duncanbeevers duncanbeevers requested a review from a team as a code owner December 13, 2024 21:39
Copy link

changeset-bot bot commented Dec 13, 2024

🦋 Changeset detected

Latest commit: 9b38b53

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
openapi-typescript Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@duncanbeevers duncanbeevers changed the title Use const arrays for OpenAPI enum types Fix colliding enumValues true values by using const arrays for OpenAPI enum types Dec 23, 2024
@duncanbeevers duncanbeevers changed the title Fix colliding enumValues true values by using const arrays for OpenAPI enum types Fix colliding enumValues exports Dec 24, 2024
@drwpow drwpow changed the base branch from main to 8.x January 3, 2025 21:04
Copy link
Contributor

@drwpow drwpow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the other PR—would love to use this as the base for 8.x. Thank you for adding such a big improvement to the library!

This can merge into 8.x with the minor conflict resolved (apologies if I merged them out-of-order)

@duncanbeevers duncanbeevers force-pushed the enum-naming-conflicts branch from 0838a67 to c0a74fe Compare January 3, 2025 23:41
duncanbeevers and others added 5 commits January 3, 2025 15:42
* Add schema to postTransform options

Closes openapi-ts#2013

* fixup! Add schema to postTransform options

Closes openapi-ts#2013

* fixup! Add schema to postTransform options

Closes openapi-ts#2013

* fixup! Add schema to postTransform options
* Only check GitHub token for docs build when update needed

* Add Netlify badge

* Use link
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@duncanbeevers duncanbeevers force-pushed the enum-naming-conflicts branch from c0a74fe to 5b0cbf9 Compare January 3, 2025 23:43
Preserve capitalization of named, exported enum values.
Type exported const values as const, instead of their location within the operations or components schemas.
Derive and export types for enum values from concrete values in const arrays.
Use derived enum value types in operations and components schemas.
Use non-conflicting variable names for composed OpenAPI enums (anyOf: [enum1, enum2])
@duncanbeevers duncanbeevers force-pushed the enum-naming-conflicts branch from 5b0cbf9 to 9b38b53 Compare January 3, 2025 23:52
@duncanbeevers
Copy link
Contributor Author

I rebased this off of main, then off of 8.x, so it might be a little squirrely, but I think I got all the right commits in there.

@drwpow
Copy link
Contributor

drwpow commented Jan 4, 2025

I rebased this off of main, then off of 8.x, so it might be a little squirrely, but I think I got all the right commits in there.

Ah yeah no worries. 8.x may diverge from main for a little bit and that’s OK. But also being a major release it’s expected. Thanks again!

@drwpow drwpow merged commit c8708e2 into openapi-ts:8.x Jan 4, 2025
7 checks passed
@duncanbeevers duncanbeevers deleted the enum-naming-conflicts branch January 6, 2025 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants