Emotion: Typing of props in restyled components is broken in latest build

Created on 13 Jun 2018  ·  28Comments  ·  Source: emotion-js/emotion

Big changes were made to the typings in the last few releases.
I tried to upgrade from 9.1.3 to 9.2.3, where it appears that inference of props types is broken.

  • emotion version: 9.2.3
  • react version: 16.4.0
  • typescript version: 2.9.1

Relevant code.

import styled from 'react-emotion';
import React from 'react';

interface MyProps {
  className?: string;
  someBehavior: boolean;
}

const MyComponent: React.SFC<MyProps> = ({className}) => <div className={className} />

const MyStyledComponent = styled(MyComponent)`
  border: ${props => props.someBehavior ? 1 : 0}px;
`;

Results in this error:

Property 'someBehavior' does not exist on type '{ theme: any; }'.
TypeScript discussion stale

Most helpful comment

@mvestergaard Furthermore, if you use TS >= 2.9, you can extract :MyProps as a type parameter like

const MyStyledComponent = styled(MyComponent)<MyProps>`
  border: ${props => props.someBehavior ? 1 : 0}px;
`;

All 28 comments

@mvestergaard I removed that inference because of withComponent.
Let's suppose that you have following code.

interface Props0 {
  typing: number;
  world: number;
}
declare const Component0: React.SFC<Props0>;
interface Props1 {
  typing: { [key: string]: string };
  safe: boolean;
}
declare const Component1: React.SFC<Props1>;

const StyledComponent0 = styled(Component0)`
  left: ${props => props.typing}px;
  top: ${props => props.world}px;
`
const StyledComponent1 = StyledComponent0.withComponent(Component1);

Now, what is the correct type for StyledComponent1? At least I cannot find the answer.

Without that inference, in other words, without _dependency on props of specific component_, we can determine type of props deterministic way.

So you're saying that you intend it to work this way?

Having to write this type of thing everywhere becomes tedious fast.

const MyStyledComponent = styled(MyComponent)`
  border: ${(props: MyProps) => props.someBehavior ? 1 : 0}px;
`;

Isn't the code i wrote a more common use case than .withComponent?
Should you really make that sacrifice?

This is borderline incentive for me to move back to styled-components if you're saying this is how you intend it to work.

@mvestergaard So, what's your answer? What is the correct type for StyledComponent1? Is what you mean just giving up to give correct type for components with .withComponent?

@mvestergaard Furthermore, if you use TS >= 2.9, you can extract :MyProps as a type parameter like

const MyStyledComponent = styled(MyComponent)<MyProps>`
  border: ${props => props.someBehavior ? 1 : 0}px;
`;

My answer is that you can't infer that. At best it would be Props0 & Props1
But my counter-question is, should you really sacrifice inference in the normal use case, because you can't make it work for .withComponent?

@mvestergaard And more importantly, IMO, that's not the most frequent use case, since you usually don't need to forward props for styling to internal component.

At least I use something like this.

interface MyProps {
  className?: string;
}
const MyComponent: React.SFC<MyProps> = ({className}) => <div className={className} />

interface StyledProps {
  someBehavior: boolean;
}
const MyStyledComponent = styled(MyComponent)<StyledProps>`
  border: ${props => props.someBehavior ? 1 : 0}px;
`;

@mvestergaard But, OK. If you truly want our typings to support your use case, let me have time for it. I need to think about it.

My opinion is that if possible, the code you write in typescript should not be different from what it would be in javascript. You should not need to tack on types where it can be inferred. But it seems we don't share that opinion.

The problem here is mostly that you've changed something that made existing code which worked just fine, suddenly have errors.

This is a minor thing, not related to you, but there's also the problem that the styled-components vscode extension doesn't support this syntax:

const MyStyledComponent = styled(MyComponent)<MyProps>`
  border: ${props => (props.someBehavior ? 1 : 0)}px;
`;

@mvestergaard Oh, really? Thank you for information. I didn't know that. (I don't use vscode)

@mvestergaard So, in summary, your opinion is, when there's no type parameter, props for styled component should be same with props of original component, is this right?

Yes, that was the behavior prior to the changes.

@mvestergaard ~I cannot find a way except breaking withComponent type (and I used it quite lot in my commercial project).
Let me ask others about this.
mitchellhamilton, tkh44, How do you think about this typing issue?
I mean, type inferencing for styled case is important, or preserving withComponent type is important? Which one is more frequent use case of emotion users?~

I think I found a way. I will send a PR for this.

@mvestergaard I just opened #722.

I have a similar issue in my codebase, previously was used react emotion 9.1.3 with typescript 2.8.3 and typing my styled components like this:

import styled from 'react-emotion';
import { IThemeProps } from 'types';

export interface IProps extends IThemeProps {
  fullWidth?: boolean;
  centered?: boolean;
}

export const Button = styled<IProps, 'button'>('button')`
  background-color: ${props => props.theme.color.brandPrimary};
  color: ${props => props.theme.color.black};
  min-width: ${({ fullWidth }) => (fullWidth ? 'auto' : '250px')};
  width: ${({ fullWidth }) => (fullWidth ? '100%' : 'auto')};
  margin: ${({ centered }) => (centered ? '0 auto' : 0)};
`;

and it works pretty well. Now I updated to the latest version of react-motion and typescript 2.9 and every component is complaining about improper types. Also tried the new typing for template literals in TS 2.9 but as @mvestergaard wrote above this breaks syntax highlighting in vs code. Is there any other solution to fix this?

If syntax breaking is the biggest issue, I will check the highlighting package and send a PR for updating.
Is what you guys use vscode-styled-component?

It isn’t the main problem. It just makes the workaround less viable.
lør. 16. jun. 2018 kl. 16.58 skrev Junyoung Clare Jang <
[email protected]>:

If syntax breaking is the biggest issue, I will check the highlighting
package and send a PR for updating.
Is what you guys use vscode-styled-component?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/emotion-js/emotion/issues/721#issuecomment-397817740,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB7xOig7wWCBPNrQ0Fcv9pap27cwznEXks5t9R0ggaJpZM4UmOMQ
.

@mvestergaard Oh, sorry. I know your biggest problem is adding type parameter itself.
Who I want to ask about it was @patrykkarny. It's my fault in my wording. Sorry for confusion.

@Ailrun yep, I'm using vscode-styled-component, the spike of it I need to change types in almost 200 components in my codebase

@patrykkarny OK. I agreed with you that's not small change.
However, I just now notice that your problem is different from what @mvestergaard says, since what you say about is 'Emotion typings change its type parameter structure!', and what he says is 'Emotion typings now cannot infer a type parameter from its arguments!'.
Those are completely different problems in terms of typings... Could you open a new issue?

@Ailrun your right I will open another issue for this tomorrow because it's late here

@patrykkarny However, I believe that your problem is also resolved with #722.
I know that PR is now somewhat mixed... but those changes affect each other :(

@Ailrun #722 should fix my incompatibility issue :) so should I wait until this PR will be merged or create another issue as you suggested? :)

Please wait, what I said is my mistake. I thought I can resolve both things separately.

It's ok :) Thanks for your work and quick answers 👍

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jaredLunde picture jaredLunde  ·  22Comments

jamiewinder picture jamiewinder  ·  24Comments

Andarist picture Andarist  ·  54Comments

kylegach picture kylegach  ·  63Comments

Slapbox picture Slapbox  ·  24Comments