paypal / glamorous

DEPRECATED: 💄 Maintainable CSS with React
https://glamorous.rocks
MIT License
3.64k stars 208 forks source link

TypeScript error when extending a component "withProps" #398

Closed tikotzky closed 6 years ago

tikotzky commented 6 years ago

Relevant code.

type H2Props = { color: string };

const H2 = glamorous.h1<H2Props>(({ color }) => ({ color }));
const GreenH2 = H2.withProps({ color: 'green' });

<GreenH2>Compiler error 😞</GreenH2>

What you did: Created a component and then set default props using withProps

What happened: The TS compiler still requires the props to be passed even though there are default props set withProps

monosnap 2018-03-19 23-32-01

Reproduction:

https://codesandbox.io/s/z7y906l2p

Problem description: TS is requiring props which should be optional since defaults were set using withProps

Suggested solution: Not sure...

luke-john commented 6 years ago

Hey @tikotzky thanks for the detailed error report, as a short term "fix" I'd suggest asserting the component created via withProps to the correct type.

https://codesandbox.io/s/q39q1ljj19

If no one else has, I'll try and have a deeper look and see how this may be resolved on the weekend.

urbackmk commented 6 years ago

+1 running into the same problem

marzelin commented 6 years ago

origins of this bug

The relevant code for this issue is:

export interface GlamorousComponentFunctions<ExternalProps, Props> {
  ...
  withProps: <DefaultProps extends object>(
    props: DefaultProps,
  ) => GlamorousComponent<ExternalProps & Partial<DefaultProps>, Props>
}

where: ExternalProps are extra props that a component needs. In your expample it is H2Props. DefaultProps are props that we provide with withProps method. Those props should be opional in the returned component. Props are CSS props (not relevant in this case)

As we can see the props for the returned component are of type ExternalProps & Partial<DefaultProps>. In the example it translates to: H2Props & Partial<H2Props> which gives {color: string} & {color?: string}. This unfortunately is equal to { color: string } as the stricter constraints take precedence which makes providing color prop to GreenH2 a requirement. This of course doesn't express the behavior of the code where color should be optional.

how to fix this

To fix this we need to make the props provided in DefaultProps object optional. The way to do this is to exclude props in DefaultProps from ExteralProps. The proper type for the returned component should be:

GlamorousComponent<
    Pick<ExternalProps, Diff<keyof ExternalProps, keyof DefaultProps>> &
      Partial<DefaultProps>,
    Props
  >;

Diff returns keys that are in ExternalProps but not in DefaultProps

In 2.8 version of typescript we can also use built-in types instead of hacky Diff:

GlamorousComponent<
    Pick<ExternalProps, Exclude<keyof ExternalProps, keyof DefaultProps>> &
      Partial<DefaultProps>,
    Props
  >;
keesvanlierop commented 6 years ago

This would be really nice.

kentcdodds commented 6 years ago

This project is now in an unmaintained status. Please see the README for more information.