Skip to content

Replace invalid props example #619

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 3 commits into from
Apr 12, 2023
Merged

Replace invalid props example #619

merged 3 commits into from
Apr 12, 2023

Conversation

filiptammergard
Copy link
Collaborator

@filiptammergard filiptammergard commented Apr 6, 2023

The previous example was titled "Props: Must Pass Both" with this:

type OneOrAnother<T1, T2> =
  | (T1 & { [K in keyof T2]?: undefined })
  | (T2 & { [K in keyof T1]?: undefined });

type Props = OneOrAnother<{ a: string; b: string }, {}>;

const a: Props = { a: "a" }; // error
const b: Props = { b: "b" }; // error
const ab: Props = { a: "a", b: "b" }; // ok

Which is just weird to me, because if you really meant "must pass both", you can just do this:

interface Props {
  a: string
  b: string
}

const Both = (props: Props) => {
  return <>both</>
}

const Component = () => (
  <>
    <Both /> {/* error */}
    <Both a="" /> {/* error */}
    <Both b="" /> {/* error */}
    <Both a="" b="" /> {/* ok */}
  </>
)

...which is not very interesting and nothing to showcase as an advanced pattern. If we make it "Pass nothing or all" it gets a little bit more interesting at least!

I think the current example is actually trying to do "Pass one or the other but not both", but that's already covered in #620.

@filiptammergard filiptammergard force-pushed the nothing-or-all branch 5 times, most recently from 89c8aac to e773f0f Compare April 6, 2023 06:51
@filiptammergard filiptammergard marked this pull request as ready for review April 6, 2023 07:24
@filiptammergard filiptammergard requested a review from eps1lon April 6, 2023 07:24
| (T1 & { [K in keyof T2]?: undefined })
| (T2 & { [K in keyof T1]?: undefined });
interface Nothing {
[key: string]: never; // needed because an empty interface is equivalent to `{}`
Copy link
Member

Choose a reason for hiding this comment

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

We should link to a section that explains why {} is bad. It's not intuitive why {} is bad.

Also: should this be [key: keyof any]?

Would also be nice to check what others recommend instead of {}. I've seen Record<keyof any, never> as well so if that's more popular, I'd stick with that to avoid fragmentation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I dug a little deeper and this is a bit of a rabbit hole. The most commonly used way seems to be Record<string, never>, and that's also what typescript-eslint recommends as default:

If you want a type meaning "empty object", you probably want Record<string, never> instead.

But it does not seem like this is recommended by the TypeScript team, since never semantically means something else: microsoft/TypeScript#47486 (comment)

I added dedicated sections about these object types to make it easier to link to them. I also added alternative approaches that might be better to avoid having to rely on typing an empty object.

What do you think?

@filiptammergard filiptammergard merged commit 9985de7 into main Apr 12, 2023
@filiptammergard filiptammergard deleted the nothing-or-all branch April 12, 2023 10:48
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