Skip to content

Fix one or the other prop pattern example #620

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

Merged
merged 1 commit into from
Apr 29, 2023
Merged

Fix one or the other prop pattern example #620

merged 1 commit into from
Apr 29, 2023

Conversation

filiptammergard
Copy link
Collaborator

@filiptammergard filiptammergard commented Apr 6, 2023

Fixes #618.

See TypeScript Playground.

Is there some more idiomatic way of achieving this?

@filiptammergard filiptammergard force-pushed the never branch 3 times, most recently from 2018c5c to 9fce27d Compare April 6, 2023 07:19
@filiptammergard filiptammergard requested a review from eps1lon April 6, 2023 07:24
@filiptammergard filiptammergard marked this pull request as ready for review April 6, 2023 07:24
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

I was confused by this as well. I though this just worked but it's not JSX related but also for normal functions as well (try MyComponent({ foo: string, bar: string }).

Could you check if there's an open TypeScript issue around this that we can link to?

@filiptammergard
Copy link
Collaborator Author

I was confused by this as well. I though this just worked but it's not JSX related but also for normal functions as well (try MyComponent({ foo: string, bar: string }).

Could you check if there's an open TypeScript issue around this that we can link to?

I looked around a bit, but couldn't find an issue or documentation describing this. But it makes sense, since TypeScript is a structural type system, you're describing minimal rather than exact requirements. For it to actually disallow one of them, a discriminating union is needed.

Added an explanation around it, and also an alternative using a discriminant prop.

@filiptammergard filiptammergard requested a review from eps1lon April 28, 2023 09:43
@eps1lon
Copy link
Member

eps1lon commented Apr 28, 2023

Yeah I saw the exchange on twitter and it kinda makes sense why TS just gives up here.

@filiptammergard filiptammergard merged commit 90442d2 into main Apr 29, 2023
@filiptammergard filiptammergard deleted the never branch April 29, 2023 05:04
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.

"Props: One or the Other but not Both" does not work
2 participants