Add withTheme definition

[patrick91]

See #483

[Igorbek]

Thank you, I also recently found that typings did not cover all the API.
In addition to inline comments, there's a definition of the styled-component module (with theme type parameter), where need to add this method also.

[Igorbek]

I don't think PureComponent should be here. PureComponent is an instance, but here must be a constructor.
What the use case required this?

[patrick91]

I saw how withRouter was implemented in React Router, they were also using PureComponent. I'll remove this :)

[Igorbek]

1. it does not reflect that the resulting component does not have theme prop
2. component parameter must have a theme prop
3. has no strong type theming support (theme prop must be of the same type as theme)
4. we do not need to capture component type and return it, we only need to capture props type and always return ComponentClass

[patrick91]

1. what's the point for this? you need to specify the theme prop anyway, like this:

class MyComponent extends React.Component<ThemeProps<{}>, {}> {
  render() {
    const { theme } = this.props;

    console.log("Current theme: ", theme);

    return <h1>Hello</h1>;
  }
}

export default withTheme(MyComponent);

2. yeah, that would be helpful, will change :)
3. same as 1
4. will change

[Igorbek]

I'd suggest something like this here:

export function withTheme<P extends { theme?: T; }, T>(component: Component<P>): ComponentClass<rest(P, 'theme')>;

where rest(P, 'theme') is a new type operator that is going to be landed with TS smth like 2.3, unfortunately (microsoft/TypeScript#13470).
I do not have any other ideas how to express removing a property from a generic type. Do you have any?

[patrick91]

Why would you need to remove the property?

[Igorbek]

An inner class is required to have theme prop and an implementer may want to have it required:

interface MyComponentProps {
  theme: MyTheme; // I want have this required
  text: string;
}
class MyComponent extends React.Component<MyComponentProps, {}> {
  ...
}

<MyComponentProps theme={theme} text={text} /> // ok
<MyComponentProps text={text} /> // error, I can't use without theme

so after applying the withTheme, the component will lose theme prop:

interface MyThemedComponentProps {
  text: string;
}
const MyThemedComponent: ComponentClass<MyThemedComponentProps> = withTheme(MyComponent);

<MyThemedComponent text={text} /> // now that is ok
<MyThemedComponent theme={theme} text={text} /> // and now that is an error, because there's no theme prop anymore

[Igorbek]

How it going, @patrick91 ? Do you need any help?

[patrick91]

I'm pretty busy these days! I'll try to find some time soon :)

[patrick91]

@Igorbek I don't see any way to remove the theme prop any than using rest. Also we need to specify the theme prop in the component interface, otherwise we won't be able to use use it (see: http://s.patrick.wtf/FSirCmZ9dz) so I was thinking to require the them prop on the component. What do you think?

[patrick91]

I can't mark the theme prop as required, it will complain when using the component.
Let me know if you have any other feedbacks :)

[Igorbek]

yes, I've been thinking about this and haven't come up with something useful as well.
We'll be waiting for better HOC support in typescript

[mxstbr]

LGTM, let's shipit?

[patrick91]

Yup!

On Wed, 1 Mar 2017 at 07:38, Max Stoiber ***@***.***> wrote: ***@***.**** approved this pull request. LGTM, let's shipit? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#521 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAotlaZH5qPGmH6s-rrK5-UipKRTtPJMks5rhSB9gaJpZM4MGM5u> .

-- Patrick