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

Make and and or return type NonNullable #191

Closed
wants to merge 1 commit into from

Conversation

SergeAstapov
Copy link
Collaborator

No description provided.

@SergeAstapov SergeAstapov marked this pull request as ready for review September 2, 2023 13:44
Comment on lines -15 to +16
@andArg={{or (hash) (array)}}
@orArg={{and (hash) (array)}}
@andArg={{or this.objectOrUndefined this.arrayOrUndefined}}
@orArg={{and this.objectOrUndefined this.arrayOrUndefined}}
Copy link
Contributor

@Techn1x Techn1x Sep 4, 2023

Choose a reason for hiding this comment

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

@andArg={{or ...}}
@orArg={{and ...}}

I don't quite understand the test but, are these the wrong way around?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, this test case needs updates to better reflect use real world scenarios.
Will do the same prob after #196

return new Date().getMonth() === 7 ? { foo: 'bar' } : undefined;
}

get arrayOrUndefined(): object | undefined {
Copy link
Contributor

Choose a reason for hiding this comment

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

this isn't an array 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeap! it is. I had a huge doubts about this approach in the first place and more I think about it, the more I get confident this is totally wrong.
I think #196 if the way to go (just need to make CI happy)

@SergeAstapov SergeAstapov deleted the non-nullable-or-and branch September 7, 2023 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants