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

Failing test for and / or types #193

Closed
wants to merge 2 commits into from

Conversation

Techn1x
Copy link
Contributor

@Techn1x Techn1x commented Sep 4, 2023

Just wanted to create this failing types test, as the v4 release didn't quite nail the and / or helper types

This might be partially resolved by #191 ? But I am not sure.

We probably want some return types like this (examples adapted from this example microsoft/TypeScript#31579 )

type Or<A extends boolean, B extends boolean> = A extends true
    ? A // A was truthy, returns itself
    : B extends true
        ? B // B was truthy, returns itself
        : B // last item in logical OR returns itself even if falsey
// for multiple args
type Or3<A extends boolean, B extends boolean, C extends boolean> = Or<A, Or<B, C>>

Comment on lines 14 to +16
<AndOrTypeChecking
@andArg={{or (hash) (array)}}
@orArg={{and (hash) (array)}}
@andArg={{and (hash) (array)}}
@orArg={{or (hash) (array)}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seemed like these args were using the wrong helpers ? maybe?

Comment on lines +17 to +20
{{!-- and returns the last truthy item --}}
@andFallbackArg={{and true "string"}}
{{!-- or returns the first truthy item --}}
@orFallbackArg={{or undefined "string"}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both of these args only accept strings.

But the types errors indicate that the and/or helpers are trying to provide them
boolean | string
undefined | string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might seem that these are pedantic or not realistic, but I'm trying to cover a scenario like;

teacherId: string | undefined // may or may not be set
AllTeachersOptionId: "all-teachers" // always set

template
@teacherId={{or this.teacherId AllTeachersOptionId}}

@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 4, 2023

This PR can be used or closed, I don't mind.

@SergeAstapov
Copy link
Collaborator

this is fixed in #196/#199

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