Skip to content

Add ExtractStrict and ExcludeStrict types #291

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

Closed
wants to merge 1 commit into from

Conversation

Vages
Copy link

@Vages Vages commented Oct 13, 2021

Solves #222

I could not figure out how to make test cases to demonstrate that these types fail when you supply an ExcludedUnion or Union that includes an element which is not found in Type. If someone knows how, a contribution is very welcome!

@sindresorhus
Copy link
Owner

Thanks for contributing these types. Make sure you read and adhere to https://github.com/sindresorhus/type-fest/blob/main/.github/contributing.md

@sindresorhus
Copy link
Owner

You also need to add the types to the readme.

@@ -0,0 +1,18 @@
/**
Constructs a type by excluding from Type all union members that are assignable to ExcludedUnion.
Copy link
Owner

Choose a reason for hiding this comment

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

I think this description could be written a little bit clearer.

@sindresorhus
Copy link
Owner

I could not figure out how to make test cases to demonstrate that these types fail when you supply an ExcludedUnion or Union that includes an element which is not found in Type. If someone knows how, a contribution is very welcome!

I think this would be a requirement for this to be merged as otherwise it's not proven that these types actually work, even if they do, they might accidentally regress in the future. Did you try using expectError?

@Vages
Copy link
Author

Vages commented Nov 8, 2021

I think this would be a requirement for this to be merged as otherwise it's not proven that these types actually work

I agree one hundred percent 😅

Did you try using expectError?

Yeah, but I did not succeed after trying for 30 minutes. I'll just try again; I'm sure I'll manage if I give it enough time.

I'll come back to fix this this sometime within the next few months.

//=> 'xxxl' | 'xxl' | 'xl' | 'l';
```

@category Utilities
Copy link
Owner

Choose a reason for hiding this comment

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

I think we should also have a category for utilities that are better versions of builtins. Maybe called Improved builtin?

Suggested change
@category Utilities
@category Improved builtin

Copy link
Author

Choose a reason for hiding this comment

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

I think that's a great idea. 👍

@Vages
Copy link
Author

Vages commented Jan 10, 2022

Please excuse me for taking so long to incorporate your suggestions, @sindresorhus. I'm currently finishing a book, and I do not expect to have time to finish this PR before the middle of February.

@sindresorhus
Copy link
Owner

and I do not expect to have time to finish this PR before the middle of February.

No worries. I'll leave this open until you have some time.

@sindresorhus
Copy link
Owner

Friendly bump :)

@sindresorhus
Copy link
Owner

#222 (comment)

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