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

Typescript issue with passed props on extended components #672

Closed
esamattis opened this issue May 24, 2018 · 11 comments
Closed

Typescript issue with passed props on extended components #672

esamattis opened this issue May 24, 2018 · 11 comments

Comments

@esamattis
Copy link

Hi! since glamorous is being deprecated I'm trying to migrate to emotion but I'm stuck with Typescript types.

  • emotion version: 9.1.3
  • react version: 16.3.2
  • typescript version: 2.8.3

My app is built using glamorous by extending every component from a custom View component (inspired by React Native).

export const View = styled.div({
    display: "flex",
    position: "relative",
    boxSizing: "border-box",
    flexDirection: "column",
});

const RedView = styled(View)({
    backgroundColor: "red",
});

This works great but often I do use props passing and type them too. This is no trouble for glamorous typescript typings but react-emotion seem to trip on it:

const MaybeRed = styled<{red: boolean}, "div">(View)(props => ({
    backgroundColor: props.red ? "red" : "white",
}));

image

The full type error is here:

[ts]
Argument of type 'StyledComponent<{}, any, ElementProps<"div">>' is not assignable to parameter of type 'Component<{ red: boolean; }>'.
  Type 'StyledComponent<{}, any, ElementProps<"div">>' is not assignable to type 'StatelessComponent<{ red: boolean; }>'.
    Types of property 'propTypes' are incompatible.
      Type 'ValidationMap<ElementProps<"div">> | undefined' is not assignable to type 'ValidationMap<{ red: boolean; }> | undefined'.
        Type 'ValidationMap<ElementProps<"div">>' is not assignable to type 'ValidationMap<{ red: boolean; }> | undefined'.
          Type 'ValidationMap<ElementProps<"div">>' has no properties in common with type 'ValidationMap<{ red: boolean; }>'.
const View: StyledComponent<{}, any, ClassAttributes<HTMLDivElement> & HTMLAttributes<HTMLDivElement> & {
    innerRef?: string | ((instance: HTMLDivElement | null) => any) | RefObject<HTMLDivElement> | undefined;
}>

And the error on props is just Parameter 'props' implicitly has an 'any' type..

Also glamorous does not require the second "div" type parameter, wonder why emotion does?

@vadistic
Copy link

vadistic commented May 25, 2018

Hello @epeli, as far as I know emotion styled interface require two parameters, because it supports also tagged template strings.

Until recently it was impossible to infer types passed to tagged templates via generics (TS: Support passing generics to tagged template string #11947 - PR got merged to roll out in typescript 2.9, let's see how it'll go). It's mentioned in Emotion docs.

About your example:
The parameters of styled interface are <StyledProps, Component<StyledProps>> so in your case

type StyledProps = React.HTMLAttributes<HTMLDivElement> /* Because it's a div and acepts html props*/ 
                     & ViewProps /* Any additional props you  may want to pass to View (here's void)*/
                     & InjectedProps /* Your injected props (here's red?) */

// Component<StyledProps> is not `div` anymore. It's StyledComponent type - in your case with additional injected props
// which you can just declare as React.ComponentType<StyledProps>

const MaybeRed = styled<StyledProps, React.ComponentType<StyledProps>>(View)(props => ({
  backgroundColor: props.red ? "red" : "white"
}));

Here's CodeSandbox

It's a bit verbose, but if you reuse View component all the time - just declare ViewProps with HTML Atributess once and reexport it.

Also there is currently an open PR to improve Emotion typings, so it's gonna get only better :)

Btw. I kind of new to TypeScript, so please correct me if I'm spreading nosense🙃

@Ailrun
Copy link
Member

Ailrun commented May 25, 2018

@epeli Hi, I'm from glamorous.
In glamorous, you don't need second parameter since glamorous infer it in other level of generic. I mean,

type Fun<First, Second> = (a: First, ar: Second) => boolean;

interface ExampleBase<First> {
  <Second>(x: First): Fun<First, Second>;
}

interface Example extends ExampleBase<'div'>, ExampleBase<'a'> {}

declare const ex: Example;

ex<{ x: number }>('div'); // First is infered to 'div'
ex<{ x: number }>('a'); // First is infered to 'a'

However, current emotion type does not use this 'two levels of generic', so it's impossible to infer second type parameter when you give the first parameter to styled.
My improved type does not include first parameter, and if you want to use custom props you should refer it before styling, like

emotion('div')<{ red: boolean }>((props) => ({
  backgroundColor: props.red ? 'red' : 'white',
});

If you think this looks awful and glamorous approach is better, I'm willing to change type structure.

@esamattis
Copy link
Author

@vadistic

Hello @epeli, as far as I know emotion styled interface require two parameters, because it supports also tagged template strings.

Until recently it was impossible to infer types passed to tagged templates via generics (TS: Support passing generics to tagged template string #11947 - PR got merged to roll out in typescript 2.9, let's see how it'll go). It's mentioned in Emotion docs.

I wonder if it would be possible to do completely different typings when it is used as tagged template and when used as function call with an object param? Ie. it would work like glamorous when the object is passed. Should be possible at least with conditional types or maybe even just with function overrides?

@Ailrun

If you think this looks awful and glamorous approach is better, I'm willing to change type structure.

If works when extending existing component created by emotion personally I'm cool with it as it's pretty irrelevant syntastical difference, but glamorous style would make it easier to migrate from it. I think there are quite a people few coming from glamorous now as it is unmaintained.

Is there a way I could test those types?

@Ailrun
Copy link
Member

Ailrun commented May 30, 2018

@epeli

  1. Giving different typings for function call with object and for tagged template is possible (since TemplateStringsArray passed to function only if function is used as tagged template).
  2. Could you elaborate what you meant by 'test those types'? It's hard to understand...

@esamattis
Copy link
Author

You mention "My improved type does not include first parameter" so just assumed that you have made a pr or something improve ts typings in emotion.

@Ailrun
Copy link
Member

Ailrun commented May 30, 2018

@epeli Oh, the typings for react-emotion are in #678, and the typings for create-emotion-styled are already merged.

@esamattis esamattis mentioned this issue Jun 6, 2018
2 tasks
@esamattis
Copy link
Author

Woo, this is fixed by #678. Thanks!

@yahyavi
Copy link

yahyavi commented Jun 9, 2018

Hey, ran into the same problem. Is this release being published soon?

@emmatown
Copy link
Member

emmatown commented Jun 9, 2018

@yahyavi It's already been published. You probably need to update to react-emotion@9.2.1

@yahyavi
Copy link

yahyavi commented Jun 9, 2018

@mitchellhamilton Thanks. I tried 9.2.1, but it does not compile for me. I thought this is resolved in 9.2.2 that is tagged as released but not pushed to npmjs? The last version that works for me is 9.1.3.
(I am using typescript 2.8.3)

@Ailrun
Copy link
Member

Ailrun commented Jun 9, 2018

@yahyavi Could you check the document? In emotion@9.2.1, we changed typings so it requires different type parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants