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

Fix got.mergeOptions(...) types #953

Merged
merged 4 commits into from Dec 6, 2019
Merged

Fix got.mergeOptions(...) types #953

merged 4 commits into from Dec 6, 2019

Conversation

scttcper
Copy link
Contributor

@scttcper scttcper commented Dec 1, 2019

Adds test coverage for mergeOptions usage similar to that in the readme.

Not passing because I'm not sure what the fix should be, willing to make the change. Partial?

@szmarczak
Copy link
Collaborator

The fix is to replace

mergeOptions<T extends PartialDeep<Options>>(...sources: T[]): T;

with

	mergeOptions(...sources: Options[]): NormalizedOptions;

I forgot to update it in #921

I'm not sure how to test the types though. Should we use tsd?

/cc @sindresorhus

@szmarczak szmarczak changed the title add mergeOptions test Fix got.mergeOptions(...) types Dec 2, 2019
@sindresorhus
Copy link
Owner

I'm not sure how to test the types though. Should we use tsd?

tsd is meant to test TypeScript definition files for JS projects.

If we really need to test the actual types, we could use something like this:

type AssertEqual<T, Expected> =
  T extends Expected
  ? (Expected extends T ? true : never)
  : never;

But I don't think it's needed in this case.

@sindresorhus sindresorhus merged commit b962d08 into sindresorhus:master Dec 6, 2019
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