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

Error using withStyles as decorator in TypeScript #8059

Closed
sbking opened this issue Sep 5, 2017 · 10 comments
Closed

Error using withStyles as decorator in TypeScript #8059

sbking opened this issue Sep 5, 2017 · 10 comments

Comments

@sbking
Copy link

sbking commented Sep 5, 2017

Problem description

The withStyles function requires the wrapped component type to have a required classes prop, but the returned decorated class has an optional classes prop. This causes the setState method signatures to differ, so TypeScript throws a compile error.

Steps to reproduce

Minimal code example:

interface ExampleClasses {
  root: string
}

interface ExampleProps {
  text: string
}

@withStyles<ExampleProps, ExampleClasses>({
  root: {
    color: 'blue'
  }
})
export default class Example extends Component<ExampleProps & { classes: ExampleClasses }, {}> {
  render (): JSX.Element | null | false {
    return <p className={this.props.classes.root}>{this.props.text}</p>
  }
}

Compiler error:

error TS1238: Unable to resolve signature of class decorator when called as an expression.
  Type 'ComponentClass<ExampleProps & StyledComponentProps<ExampleClasses>>' is not assignable to type 'typeof Example'.
    Type 'Component<ExampleProps & StyledComponentProps<ExampleClasses>, ComponentState>' is not assignable to type 'Example'.
      Types of property 'setState' are incompatible.
        Type '{ <K extends never>(f: (prevState: ComponentState, props: ExampleProps & StyledComponentProps<Exa...' is not assignable to type '{ <K extends never>(f: (prevState: {}, props: ExampleProps & { classes: ExampleClasses; }) => Pic...'.
          Types of parameters 'f' and 'f' are incompatible.
            Types of parameters 'props' and 'props' are incompatible.
              Type 'ExampleProps & StyledComponentProps<ExampleClasses>' is not assignable to type 'ExampleProps & { classes: ExampleClasses; }'.
                Type 'ExampleProps & StyledComponentProps<ExampleClasses>' is not assignable to type '{ classes: ExampleClasses; }'.
                  Types of property 'classes' are incompatible.
                    Type 'ExampleClasses | undefined' is not assignable to type 'ExampleClasses'.
                      Type 'undefined' is not assignable to type 'ExampleClasses'.

Versions

  • Material-UI: 1.0.0-beta.8
  • React: 15.6.1
  • types/react: 15.6.1
  • TypeScript: 2.5.2
@sbking
Copy link
Author

sbking commented Sep 5, 2017

Here is the workaround I'm currently using:

interface ExampleClasses {
  root: string
}

interface ExampleProps {
  text: string
  classes: any
}

@withStyles<ExampleProps, ExampleClasses>({
  root: {
    color: 'blue'
  }
})
export default class Example extends Component<ExampleProps, {}> {
  render (): JSX.Element | null | false {
    const classes = this.props.classes as ExampleClasses
    const { text } = this.props
    return <p className={classes.root}>{text}</p>
  }
}

@marcusjwhelan
Copy link

@sbking I would keep the classes separate. Reason being is if you are using the component as a child then classes would be required to be injected unless you have a ?: although then you have to check to see if classes exists for Typescript not to complain to you since it could be undefined.

With this setup you get the autocompletion of you Styles with IStyles, only the props you want injected. And with redux you need if used you have it set up like so.

interface IStyles {
    // styles interface
}
interface IHeaderInfoStyles {
    classes: any;
}
interface IHeaderInfoProps {
    // properties resulting from dispatches
}
interface IHeaderInfoInjectedProps {
   // props injected from parent if any
}
interface IHeaderInfoDispatchProps {
    // dispatches
}
interface IHeaderInfoState {
    // your state for the class
}

type HeaderInfoProps = IHeaderInfoProps & IHeaderInfoInjectedProps & IHeaderInfoDispatchProps & IHeaderInfoStyles;

class HeaderInfo extends Component<HeaderInfoProps, IHeaderInfoState> {

export const HeaderInfo_container = withStyles<IHeaderInfoInjectedProps, IStyles>(styles)(
    connect<IHeaderInfoProps, IHeaderInfoDispatchProps, (IHeaderInfoInjectedProps & IHeaderInfoStyles)>(mapStateToProps, mapDispatchToProps)(HeaderInfo),
);

@sbking
Copy link
Author

sbking commented Sep 7, 2017

@marcusjwhelan Does that work with decorators, though? The example in my OP (with separate Props and Classes interfaces) works fine if you change it from using withStyles as a decorator to a normal function, since TypeScript then doesn't care if the resulting class has the same method signatures as the original class.

I'm slightly confused about your example, though. How would you access a JSS class from within the component?

@marcusjwhelan
Copy link

@sbking I am not exactly sure. I don't even know what JSS is. connect is from react-redux. What I was saying is yes it works but what I was saying is that in the case the class is a child component you don't want to have to pass in the classes from the parent if you want separate component withStyes classes from the parent classes.

It should work with the decorator just fine. I don't use them.

class HeaderInfo extends Component<HeaderInfoProps, IHeaderInfoState> {
    constructor(props: HeaderInfoProps) {
        super(props);
        this.state = { /* your state init */};
    }
}

I am confused by the last part of your question but you should be able to access all the properties of dispatch, styles, props, because all the interfaces were & together.

@sbking
Copy link
Author

sbking commented Sep 7, 2017

@marcusjwhelan JSS is the system Material UI 1.0 uses internally for scoped CSS classes.

Having a required classes prop for the inner component that will be wrapped seems to be the standard in Material UI:

https://github.com/callemall/material-ui/blob/v1-beta/src/Badge/Badge.js#L72

The withStyles HOC always injects the classes prop into the inner component, and then in the resulting wrapped component, classes is exposed as an optional prop for overrides. The inner component expects classes to be provided by its parent, which is almost always going to be the wrapping component provided by withStyles. In your code, I'm still not seeing an access of this.props.classes.whatever which is what is confusing me. Where do you actually access the classes provided by the withStyles HOC?

Anyway, this will still not work as a decorator, because TypeScript enforces that class decorators return a type that is compatible with the original decorated class. The withStyles HOC changes the classes prop from required to optional, which means the types of some component methods are changed and the classes are not compatible. See the following definitions:

https://github.com/callemall/material-ui/blob/v1-beta/src/styles/withStyles.d.ts#L27
^ Notice that classes is a required prop for the component that is passed to withStyles.

https://github.com/callemall/material-ui/blob/v1-beta/src/index.d.ts#L12-L16
^ But withStyles ends up returning a component that accepts classes as an optional prop.

If the inner component does not require a classes prop in Material UI version 1.0.0-beta.8, this error results:

Argument of type 'typeof Example' is not assignable to parameter of type 'ComponentType<ExampleProps & { classes: ExampleClasses; theme?: Theme<{}> | undefined; }>'.
  Type 'typeof Example' is not assignable to type 'StatelessComponent<ExampleProps & { classes: ExampleClasses; theme?: Theme<{}> | undefined; }>'.
    Type 'typeof Example' provides no match for the signature '(props: ExampleProps & { classes: ExampleClasses; theme?: Theme<{}> | undefined; } & { children?: ReactNode; }, context?: any): ReactElement<any> | null'.
src/ui/navigation/ButtonNav/LinkButton.tsx(23,37): error TS2339: Property 'classes' does not exist on type 'Readonly<{ children?: ReactNode; }> & Readonly<ExampleProps>'.
src/ui/navigation/ButtonNav/__tests__/LinkButton.test.tsx(34,9): error TS2339: Property 'href' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Example> & Readonly<{ children?: ReactNode; }> & R...'.
src/ui/navigation/ButtonNav/__tests__/LinkButton.test.tsx(43,9): error TS2339: Property 'href' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Example> & Readonly<{ children?: ReactNode; }> & R...'.

@marcusjwhelan
Copy link

marcusjwhelan commented Sep 8, 2017

@sbking in my code. You would simply use this.props.classes.whatever like normal.

I see what you mean. The decorator will not work. Why must you use a decorator? But also this should be fixed.

However. What is wrong with putting classes: any ?
This works just fine. You wouldn't be able to use this as a child component unless you expect
to pass classes in.

interface IExampleClasses {
    root: string;
}

interface IExampleProps {
    text: string;
}

interface IExampleStyles {
    classes: any;
}
type PROPS = IExampleProps & IExampleStyles;

@withStyles<PROPS, IExampleClasses>({
    root: {
        color: 'blue',
    },
})
class Example extends Component<PROPS, any> {
    public render(): JSX.Element | null | false {
        return <p className={this.props.classes.root}>{this.props.text}</p>;
    }
}

@kgaregin
Copy link
Contributor

kgaregin commented Sep 8, 2017

same issue here

Error:(33, 1) TS1238:Unable to resolve signature of class decorator when called as an expression.
  Type 'ComponentClass<StyledComponentProps<{}>>' is not assignable to type 'typeof RootPaperComponent'.
    Type 'Component<StyledComponentProps<{}>, ComponentState>' is not assignable to type 'RootPaperComponent'.
      Types of property 'render' are incompatible.
        Type '() => false | Element | null' is not assignable to type '() => Element'.
          Type 'false | Element | null' is not assignable to type 'Element'.
            Type 'null' is not assignable to type 'Element'.

@jugglingcats
Copy link

I think my issue is the same or closely related:

@withStyles(styles)
class MyComponent extends React.Component<any, any> {
    manager: any;
    ...
}

Produces:

TS1238: Unable to resolve signature of class decorator when called as an expression.
 Type 'ComponentClass<StyledComponentProps<{}>>' is not assignable to type 'typeof MyComponent'.
 Type 'Component<StyledComponentProps<{}>, ComponentState>' is not assignable to type 'MyComponent'.
 Property 'manager' is missing in type 'Component<StyledComponentProps<{}>, ComponentState>'.

I created a stackoverflow question and someone gave a good explanation of the underlying cause and a workaround which I'm using for now.

https://stackoverflow.com/questions/46118513

@sebald
Copy link
Member

sebald commented Sep 13, 2017

With the proposed changed from SO the usage as HOC would no longer work 😕
Basically, these two (basic) variants have to be supported by the typings:

With Decorator

@withStyles(styles)
class DecoratedComponent extends React.Component<
  StyledComponentProps & { classes: StyledComponentClassNames }
> {
  render() {
    const { classes, text } = this.props;
    return (
      <div className={classes.root}>
        {text}
      </div>
    );
  }
}

As Higher-Order Function

const Component: React.SFC<
  StyledComponentProps & { classes: StyledComponentClassNames }
> = ({ classes, text }) =>
  <div className={classes.root}>
    {text}
  </div>;

const StyledComponent = withStyles<
  StyledComponentProps,
  StyledComponentClassNames
>(styles)(Component);

I am not 100% sure if the issues are the same, the issue people are running into is something like this

Types of property 'render' are incompatible.
        Type '() => false | Element' is not assignable to type '() => Element'.
          Type 'false | Element' is not assignable to type 'Element'.
            Type 'false' is not assignable to type 'Element'.

which seems a common problem with Decorators and originates from the React typings (?): DefinitelyTyped/DefinitelyTyped#17161

@sebald
Copy link
Member

sebald commented Sep 13, 2017

¯_(ツ)_/¯ So ... I made a PR, which just overloads withStyles, so that it has different signatures. One which works for decorators and one that works as HOF.

Would love some feedback if this patch breaks anyones code.

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

6 participants