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

Add ExtractStrict and ExcludeStrict #38

Merged
merged 6 commits into from
Jul 18, 2019

Conversation

ekilah
Copy link
Contributor

@ekilah ekilah commented Jul 18, 2019

Fixes #37

I ran yarn format and picked up some changes in public.ts and in the index.d.ts.

Let me know if I should also include a version number change, or if you do that yourself when publishing.

(First time using dtslint, any tips welcome. I tried to do e.g.$ExpectType 'red' | 'green' | 'blue' for type PrimaryColors and it threw errors saying it got any instead. maybe this type of thing doesn't work in dtslint or I was using it wrong? edit: maybe i could try the suggestions here: microsoft/dtslint#179 if you want me to add more tests)

@ekilah
Copy link
Contributor Author

ekilah commented Jul 18, 2019

I saw the CI errors locally too:

Error: /home/travis/build/pelotom/type-zoo/types/param-n.ts:11:1
ERROR: 11:1  expect  Expected type to be:
  {}
got:
  unknown
ERROR: 13:1  expect  Expected type to be:
  {}
got:
  unknown
ERROR: 43:1  expect  Expected type to be:
  {}
got:
  unknown

(unrelated to these changes)

@pelotom
Copy link
Owner

pelotom commented Jul 18, 2019

The CI errors are due to recent changes in TypeScript that make it so unknown is used as an upper bound instead of {} when a type parameter can't be inferred. So we should just switch the expectations to unknown.

@ekilah
Copy link
Contributor Author

ekilah commented Jul 18, 2019

@pelotom seems to fail either way, using different TS versions?

@pelotom
Copy link
Owner

pelotom commented Jul 18, 2019

I pushed a fix to master. Basically we need to expect a minimum TypeScript version of 3.5 now.

@ekilah
Copy link
Contributor Author

ekilah commented Jul 18, 2019

ah, there we go, thanks. lmk if the rest looks good when you have time!

Copy link
Owner

@pelotom pelotom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good except the "Similar to Pick/OmitStrict but for types" language... I get the gist of what you're going for but all type operators are "for types". I'm not sure how to rephrase, but personally I would just drop this line because the "Similar to Exclude/Exclude but requires..." language is sufficient.

@pelotom pelotom changed the title add ExtractStrict and ExcludeStrict Add ExtractStrict and ExcludeStrict Jul 18, 2019
@ekilah
Copy link
Contributor Author

ekilah commented Jul 18, 2019

Removed. thought it'd be nice for people unfamiliar with what the difference is between Pick and Extract, but that's not really the job of this repo I guess.

@pelotom pelotom merged commit 28951c5 into pelotom:master Jul 18, 2019
@pelotom
Copy link
Owner

pelotom commented Jul 18, 2019

Thanks!

@ekilah ekilah deleted the strictExtractExclude branch July 18, 2019 23:45
@ekilah
Copy link
Contributor Author

ekilah commented Jul 19, 2019

what's your publishing schedule for npm btw?

@pelotom
Copy link
Owner

pelotom commented Jul 19, 2019

Just published.

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.

Interest in ExtractStrict?
2 participants