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

feat: add IsX/IfX types for any/never/unknown #564

Merged
merged 30 commits into from Apr 5, 2023

Conversation

tommy-mitchell
Copy link
Contributor

@tommy-mitchell tommy-mitchell commented Mar 6, 2023

Closes #129.

Adds IsAny, IfAny, etc. types:

import type {IsAny, IfAny} from 'type-fest';

type ShouldBeTrue = IsAny<any>;
//=> true

type ShouldBeFalse = IfAny<''>;
//=> false

type ShouldBeNever = IfAny<any, never, 'not-never'>;
//=> never

The IfX types are defined using the pattern mentioned in #129 (comment):

// source/if-any.d.ts

export type IfAny<T, TypeIfAny = true, TypeIfNotAny = false> = (
  IsAny<T> extends true ? TypeIfAny : TypeIfNotAny
);

Marked draft as concrete use cases are needed for documenting the types.

Info about a blocker that has been resolved

Marked draft as concrete use cases are needed for documenting the types, and a test can't pass without tsdjs/tsd#173:

// test-d/if-any.ts

expectError<IfAny>(anything);

// reports:
// test-d\if-any.ts:25:12
//   ✖  25:0   Expected an error, but found none.
//   ✖  25:12  Generic type IfAny requires between 1 and 3 type arguments.

@tommy-mitchell tommy-mitchell changed the title Add IsX/IfX types for any/never/ unknown feat: add IsX/IfX types for any/never/unknown Mar 6, 2023
@sindresorhus
Copy link
Owner

Marked draft as concrete use cases are needed for documenting the types, and a test can't pass without tsdjs/tsd#173:

tsd update is out.

source/is-if-any.d.ts Outdated Show resolved Hide resolved
source/is-if-any.d.ts Outdated Show resolved Hide resolved
@tommy-mitchell
Copy link
Contributor Author

I'm aware that the README descriptions of the IfX types don't match their doc comments. What verbiage should we go with here? I think that this library as a whole could do with more consistent definitions (maybe a xo rule or something could help with that, especially if a README generator is implemented later).

Other than that, this PR is ready for review I think.

@tommy-mitchell tommy-mitchell marked this pull request as ready for review March 8, 2023 03:40
@sindresorhus
Copy link
Owner

What are your thoughts on subsuming the if/else functionality into the Is types? Like what's proposed in #544

@tommy-mitchell
Copy link
Contributor Author

I like the explicitness of IfX<SomeType, DoThis, DoThat>, but I think it might become unwieldy if we add an IfX type for every IsX type added (for example, the IsXLiteral types).

@sindresorhus
Copy link
Owner

I think we should be consistent though. So either we add If types for everything or we don't. Otherwise, we'll end up having to make arbitrary decisions about when it makes sense to add a If type or not.

@sindresorhus
Copy link
Owner

On the other hand, I do also like the readability of If...

@tommy-mitchell
Copy link
Contributor Author

tommy-mitchell commented Mar 10, 2023

I think the only real problem comes with documentation - there's a lot more noise if every IsX type is followed by an IfX type. Maybe there could be a section that explains the differences, then each IsX type could mention an alternative IfX type.

Something like (with more explanation):


IsX vs. IfX

For every IsX type, there is an associated IfX type that can help simplify conditional types:

import type {IsAny, IfAny} from 'type-fest';

type ShouldBeTrue = IsAny<any> extends true ? true : false;
//=> true

type ShouldBeFalse = IfAny<'not any'>;
//=> false

type ShouldBeNever = IfAny<'not any', 'not never', never>;
//=> never

Type Guards

  • IsAny - Returns a boolean for whether the given type is any. (Alternative: IfAny)
  • IsNever - Returns a boolean for whether the given type is never. (Alternative: IfNever)
  • IsUnknown - Returns a boolean for whether the given type is unknown. (Alternative: IfUnknown)

@sindresorhus
Copy link
Owner

Can you do https://github.com/sindresorhus/type-fest/pull/563/files#r1127160534 in this PR?

@bjesuiter
Copy link

I'm coming from the question at Twitter by sindresorhus.

I think, I didn't get the difference you meant with IsX and IfX, yet.

My Thoughts:

Isn't IfX the more generalized variant of IsX?
As written before, IsAny seems to me like a shortcut for IfAny<true,false>.

I personally would be fine to having a single IsX type with the functionality of IfX baked in.
As

IsAny<any> => true/false 
IsAny<any, 'type-when-true','type-when-false'> 

I see a small problem in discoverability when only having the IsX type, bc. When I search for an IfX type It would not auto-import and I would've to look into docs.

But I also see the maintenance gets a little unwieldy when adding an IfX for every IsX.

Suggestion: What are your thoughts on simply having these both, but with the same functionality? IfX would then practically be an alias of IsX for all types, simply for discoverability.

@sindresorhus
Copy link
Owner

@tommy-mitchell I like your proposal. Lets go with that.

@sindresorhus
Copy link
Owner

Suggestion: What are your thoughts on simply having these both, but with the same functionality? IfX would then practically be an alias of IsX for all types, simply for discoverability.

I think it could be confusing if IfX was aliased to IsX as the user may think it's a mistake.

@tommy-mitchell
Copy link
Contributor Author

Added the readme note and moved the IsT types I added to a Type Guard category. Should we add IfT types for the other IsT types in this PR, or should that be separate?

source/if-any.d.ts Show resolved Hide resolved
@sindresorhus
Copy link
Owner

Should we add IfT types for the other IsT types in this PR, or should that be separate?

Separate

source/if-any.d.ts Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
source/if-any.d.ts Outdated Show resolved Hide resolved

If the given type `T` is `any`, the returned type is `TypeIfAny`. Otherwise, the return type is `TypeIfNotAny`. If only `T` is specified, `TypeIfAny` will be `true` and `TypeIfNotAny` will be false.

@see IsAny
Copy link
Owner

Choose a reason for hiding this comment

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

Can you check if this is linkified in VS Code? We need a syntax that like to the type correctly so users can just click through.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hovers correctly in source files, but the rendered hover text for IfAny does not link to IsAny.

Using @see {@link IsAny} does linkify in the hover text, but it seems to go to the import statement in if-any.d.ts.

Related, most type-fest types with an @see don't use the @link syntax, so they're not correct either.

Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think having @see makes it clearer.

With:

image

Without:

image

source/if-any.d.ts Outdated Show resolved Hide resolved
source/if-any.d.ts Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

In case you missed it: #564 (comment)

@sindresorhus sindresorhus merged commit 4045737 into sindresorhus:main Apr 5, 2023
7 checks passed
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.

Proposal: IfAny, IsAny, IsUnknown, etc
3 participants