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

Change spec types to allow ReadonlyArray choices #112

Merged
merged 1 commit into from
Dec 18, 2019

Conversation

beaulac
Copy link
Contributor

@beaulac beaulac commented Dec 16, 2019

In TypeScript, the current type definitions don't allow the following:

const myChoices: ReadonlyArray<string> = ['foo', 'bar'];
const myEnv = cleanEnv(process.env, { MY_ENV_VAR: str({choices: myChoices});

// Error:(...) TS4104: The type 'readonly string[]' is 'readonly' and cannot be assigned to the mutable type 'any[]'.

Given that the choices field doesn't need to be modified (only the include method is used currently), it should be allowed to be a ReadonlyArray. Generally in TS, it’s good practice to use ReadonlyArray over Array when no mutation is intended.

Any Array<T> is also assignable to ReadonlyArray<T>, so this change doesn't break any existing behaviour.

This change uses ReadonlyArray<T> instead of readonly T[] to maintain compatibility with TS <3.x.

@af af merged commit 60bab04 into af:master Dec 18, 2019
@af
Copy link
Owner

af commented Dec 18, 2019

Looks good to me, thanks!

tuannm151 pushed a commit to BSSCommerce/shopify-envalid that referenced this pull request Jul 1, 2024
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.

2 participants