fix(openapi-typescript): correct generated type when using prefixItems
#1893
Conversation
|
There was a problem hiding this comment.
Thank you for opening this PR, and fixing the generation. I agree that this is probably the more “correct” solution to what was originally implemented. But this would be a breaking change for people, because the original implementation (whether intentionally or not) essentially ignored items.
We’ll accept the PR as-implemented, but we’ll need to make this an opt-in feature flag, for now. In the next breaking release we can look into changing the default behavior to match this (which I strongly think we will). But I don’t want to release a new major version now just for this change and this change alone.
Would you be willing to put this behind an opt-in flag?
| @@ -179,7 +245,7 @@ describe("transformSchemaObject > array", () => { | |||
| { | |||
| given: { | |||
| type: "array", | |||
| items: { type: "number" }, | |||
| items: false, | |||
There was a problem hiding this comment.
This specifically is the breaking change I’m referring to—we’ll need to ensure that the original test case of items: { type: "number" } stays tested, and it generates the same output. While we’re on version 7.x this can’t change (and we can revisit it when it’s time to release 8.x).
In general, modifying an original test is a good hint that it may be a breaking change.
| @@ -45,6 +45,72 @@ describe("transformSchemaObject > array", () => { | |||
| want: `[ | |||
| number, | |||
| number, | |||
| number, | |||
| ...number[] | |||
There was a problem hiding this comment.
This other modification to an existing test may also technically be a breaking change, but I think because it’s purely additive, could be considered a bugfix. I would be OK with this going in as-is.
Changes
Closes #1892
How to Review
Note that this PR also adds support for
unevaluatedItems, which was supported in OAS 3.1.0.Checklist
docs/updated (if necessary)pnpm run update:examplesrun (only applicable for openapi-typescript)