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

full support for minItems & maxItems #477

Merged
merged 25 commits into from
Apr 8, 2021

Conversation

medfreeman
Copy link
Contributor

@medfreeman medfreeman commented Mar 24, 2021

closes #476

@horiuchi
Copy link
Owner

Thank you for your contribution! 👍
So awesome feature!

I think the strictArraySize option is unnecessary and should always be enabled. What do you think?

@medfreeman
Copy link
Contributor Author

Thanks!

The thing is this feature can complicate consumer's code a bit, because in strict mode a tuple will be incompatible with an array.
As long as you're happy with this, i'm happy either way.
There's a case for not adding too much options too!

@medfreeman
Copy link
Contributor Author

Tell me what you choose and i'll remove the option and the test infra change if necessary.
Thanks!

@horiuchi
Copy link
Owner

horiuchi commented Apr 1, 2021

I don't want to add too many options either, so please remove the options.

The complexity of the type is not a problem for me.
I'd rather have the TypeScript compiler warn us more correctly by type definition.

…it back array tests to conform w/ default behavior
… add support for optional `strict` array size test variant through `_expected_strict.d.ts` & `_config_strict.json` files
@medfreeman medfreeman changed the title full support for minItems & maxItems, strictArraySize option full support for minItems & maxItems Apr 6, 2021
…unction, add support for optional `strict` array size test variant through `_expected_strict.d.ts` & `_config_strict.json` files"

This reverts commit 4ac320e.
@medfreeman
Copy link
Contributor Author

it's ready!

@horiuchi horiuchi merged commit 82d4253 into horiuchi:master Apr 8, 2021
@horiuchi
Copy link
Owner

horiuchi commented Apr 8, 2021

@medfreeman Thank you for awesome work! 👍

@medfreeman medfreeman deleted the strict-array-size branch April 9, 2021 09:44
@crizo23
Copy link

crizo23 commented Aug 19, 2021

@medfreeman @horiuchi

The one negative I can see of the tuples is that for types defined as:
"ArrayOfFoos": { "type": "array", "items": { "$ref": "#/components/schemas/Foo" }, "minItems": 0, "maxItems": 500 }

results in:
export type ArrayOfFoos = [ Foo ?, Foo ?, Foo ?, Foo ?, ..all the way to 500 ];

resulting in a rather large type.
I understand that this provides a type that is restricted by min/max length. Was there any way to do this differently?

@horiuchi
Copy link
Owner

@crizo23 Thank you for your information. That's a problem.

One idea would be to simply revert to an unlimited number of types as before if maxItems exceeds some threshold, although it would be a loose definition of a type.
For example, if the threshold is set to 5, then up to maxItems of 5, the type will be generated as usual for [Foo, Foo, Foo, Foo, Foo] and maxItems, but when maxItems exceeds the threshold, the type will always be generated as [Foo, Foo, Foo, Foo, Foo, ... Foo].

@crizo23
Copy link

crizo23 commented Aug 20, 2021

@crizo23 Thank you for your information. That's a problem.

One idea would be to simply revert to an unlimited number of types as before if maxItems exceeds some threshold, although it would be a loose definition of a type.
For example, if the threshold is set to 5, then up to maxItems of 5, the type will be generated as usual for [Foo, Foo, Foo, Foo, Foo] and maxItems, but when maxItems exceeds the threshold, the type will always be generated as [Foo, Foo, Foo, Foo, Foo, ... Foo].

@horiuchi I don't think a "threshold" is the answer, as that would be different for anyone using the library. I more meant, is there a better/fancier way to do this in Typescript". The answer is probably "no", in that case I think there's no way around having maxItems of tuples in the array type.

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.

Feature: full support for minItems & maxItems
3 participants