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

Improve flags types to acknowledge isMultiple and isRequired options #154

Merged
merged 6 commits into from Aug 10, 2020
Merged

Improve flags types to acknowledge isMultiple and isRequired options #154

merged 6 commits into from Aug 10, 2020

Conversation

voxpelli
Copy link
Contributor

This PR fixes two things in the types of flags:

  1. Without this PR the type of an isMultiple flag is still reported to be a non-array.
  2. All types are right now reported as always being defined, even when both isRequired and default is missing. This PR adds |undefined in those cases + does so in the case where isRequired is a function, as it's kind of hard to figure out what that function will return.

As a separate commit this PR also mirrors the Typescript config from tsd + extends it slightly. It does this for two reasons:

  1. With a tsconfig.json in the project tools like VS Code can automatically configure itself correctly and provide correct feedback. Without the tsconfig.json it complained about some of the definition tests.
  2. To be able to test |undefined one needs to have strictNullChecks set, else Typescript will just ignore any undefined added in the types.

I also added in noImplicitAny and noUnusedLocals in the tsconfig.json as both of those are helpful and good practice as well.

Somewhat breaking:

The addition of |undefined to all types that are missing both isRequired and default can cause some complaints in the code for those who have strictNullChecks set to true. For everyone else it will cause no change.

These complaints are correct though, but might cause some headaches for eg. those without lock-files, where it can have their CI fail.

Without this something like VS Code can't understand what the actual config is, even if tsd itself will know
@voxpelli
Copy link
Contributor Author

Fixed the failing tests 🙈 (A downside of being used to husky, one forgets to manually run tests before push)

@sindresorhus sindresorhus changed the title Improve flags types to acknowledge isMultiple and isRequired options Improve flags types to acknowledge isMultiple and isRequired options Jul 7, 2020
@sindresorhus
Copy link
Owner

@NiGhTTraX Would you be able to give this a quick look?

index.d.ts Outdated Show resolved Hide resolved
@NiGhTTraX
Copy link
Contributor

LGTM otherwise.

@sindresorhus sindresorhus merged commit e38789f into sindresorhus:master Aug 10, 2020
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.

None yet

3 participants