-
-
Notifications
You must be signed in to change notification settings - Fork 586
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
feat: add IsX
/IfX
types for any
/never
/unknown
#564
Conversation
IsX
/IfX
types for any
/never
/ unknown
IsX
/IfX
types for any
/never
/unknown
|
I'm aware that the Other than that, this PR is ready for review I think. |
What are your thoughts on subsuming the if/else functionality into the |
I like the explicitness of |
I think we should be consistent though. So either we add |
On the other hand, I do also like the readability of |
I think the only real problem comes with documentation - there's a lot more noise if every Something like (with more explanation):
|
Can you do https://github.com/sindresorhus/type-fest/pull/563/files#r1127160534 in this PR? |
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? I personally would be fine to having a single IsX type with the functionality of IfX baked in.
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. |
@tommy-mitchell I like your proposal. Lets go with that. |
I think it could be confusing if |
Added the readme note and moved the |
Separate |
source/if-any.d.ts
Outdated
|
||
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about only using @link
? https://devblogs.microsoft.com/typescript/announcing-typescript-4-3/#jsdoc-link-tags
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case you missed it: #564 (comment) |
Closes #129.
Adds
IsAny
,IfAny
, etc. types:The
IfX
types are defined using the pattern mentioned in #129 (comment):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: