Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork552
Fix nested length-constrained array generation#2330
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
netlifybot commentedMay 21, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
👷 Deploy request foropenapi-ts pending review.Visit the deploys page to approve it
|
changeset-botbot commentedMay 21, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
|
fafb525
to19be4e4
Comparethrowandgo commentedMay 29, 2025
@duncanbeevers my team is also running into this issue. Is there any additional context we can provide to help push this through? |
Ithink#2148 should address this, but I haven't rebased it for a while. |
Uh oh!
There was an error while loading.Please reload this page.
Changes
This fixes a bug when doing nested length-constrained array generation, specifically the situation added in
options > arrayLength: true > minItems: 1, maxItems: 1; minItems: 1, maxItems: 1
. Without the change, the code would generate the following (note the extra layer of nesting):What went wrong in the old code? I think the main issue is that
itemType
could either be the actual item type, or the type of the entire array (in the case of a tuple). In the problematic nesting situation, in the outer nesting, we run the codeitemType = ts.factory.createArrayTypeNode(transformSchemaObject(schemaObject.items, options));
. The call totransformSchemaObject
returns[string]
, and thencreateArrayTypeNode
turns it into[string][]
. Finally, we go through the logic ofarrayLength
, which wraps that double nested array again as a singleton array, to become a triply-nested array.This also likely fixes a situation where length-constrained arrays were not being declared as
readonly
when the immutable flag was set - notice the old code early-returns when computing length-constrained arrays.The obvious first change is to stop doing
itemType = ts.factory.createArrayTypeNode(transformSchemaObject(schemaObject.items, options));
completely and always just doitemType = transformSchemaObject(schemaObject.items, options);
, but this runs into a problem with basic nested arrays. In the basic nested array case, in the inner runitemType
isstring[]
, which makes sense, and then we would like to in the outer run turn that it intostring[][]
. However, this does not happen - when computingfinalType
, we will skip the array-wrapping in this case, as itemType is indeed an array (it is an array because the type of the item is a nested array, but we do not know that).Fundamentally, I think the problem is that sometimes
itemType
is the type of a single item in the array, and sometimes it is the entire array, and we cannot simply check whetheritemType
is an array/tuple or not to detect that, so in the length-unconstrained array case we added a hack that then caused problems with the nested length-constrained array case.Hence, this diff changes things so that
itemType
is truly the type of the item, and hence is applicable only with arrays. Instead, we havearrayType
, which is the type of the array/tuple before potentially applying immutability. In this flow of logic, for tuple types, we simply create the tuple and stop there in thearrayType
computation. It is only for arrays that we compute theitemType
, which is truly the type of a single item in the array. Then, for length-constrained arrays we use thatitemType
to generate the array, whiule for length-unconstrained arrays we simply make an array ofitemType
.How to Review
The main thing is - are there additional missing tests, which would catch issues overlooked in this change? Also, the change in logic should hopefully be clearer in addition to fixing this specific problem.
(The main code is also easier to review when hiding whitespace)
Checklist
docs/
updated (if necessary)pnpm run update:examples
run (only applicable for openapi-typescript)