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 prefixItems / minItems / maxItems tuple generation (#2053) #2148

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

duncanbeevers
Copy link
Contributor

@duncanbeevers duncanbeevers commented Feb 10, 2025

🗣️ Discussion

This PR is an extraction of some work that has already landed in the 8.x branch. This is my first step in porting those changes to the 7.x branch.

Closes #2048

Changes

  • Simplify minItems / maxItems tuple generation
  • Introduces @total-typescript to accommodate Array.prototype.filter(Boolean) pattern
  • Extract subroutine for transforming array schema objects
  • Support items: false; prefixItems: … schemas; treat prefixItems as explicit tuple value.
  • Support items: […schemas], treating items as explicit tuple value.

How to Review

  • Ensure changes to newly-generated types are valid bug-fixes
  • Ensure old behaviors are preserved where appropriate, to break fewer things when we release
  • Ensure new behavior is gated behind --experimental-array-spread-members flag
  • Ensure ts-reset types don't affect published openapi-ts types
    • ts-reset behaviors should not be active in plain repo with openapi-ts dependency
    • Importing openapi-ts for programmatic use shouldn't start allowing ts-reset behaviors.
  • Ensure ts-reset types don't affect generated types
    • Importing openapi-typescript-generated types shouldn't start allowing ts-reset behavior.

🔧 Backwards-compatibility

In order to get a sense for the backwards-compatibility of this PR I pulled the full suite of tests from array.test.ts and ran them in main. Test output

Many test cases in this corpus cover behavior not-yet-tested in main, so this gives us a better sense of what will actually change.

Many of the new tests (expectedly) failed in this older context, and the errors fell into four categories.

  1. New behavior gated behind the new --experimental-array-spread-members flag.
  2. Mutable spread members for array schemas when immutable: true is set
  3. Mutable tuples for array schemas with prefixItems when immutable: true is set
  4. Broken tuples (original --array-length with minItems: 1 generates empty array #2048 problem)

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 February 10, 2025 18:09
@duncanbeevers duncanbeevers requested a review from gzm0 February 10, 2025 18:09
Copy link

netlify bot commented Feb 10, 2025

👷 Deploy request for openapi-ts pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit b6d6fcb

Copy link

changeset-bot bot commented Feb 10, 2025

🦋 Changeset detected

Latest commit: 8a09dea

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 Minor

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 force-pushed the fix-2048-tuples-generation branch 2 times, most recently from 3285052 to 8a09dea Compare February 15, 2025 22:05
@gzm0
Copy link
Contributor

gzm0 commented Feb 19, 2025

@drwpow you have looked at the 8.x equivalent of this. Do you also want to take this one, or shall I give it a stab to review?

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.

--array-length with minItems: 1 generates empty array
2 participants