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] Generic React ClassComponent && withStyles #14544

Closed
nihaux opened this issue Feb 15, 2019 · 7 comments
Closed

[Typescript] Generic React ClassComponent && withStyles #14544

nihaux opened this issue Feb 15, 2019 · 7 comments
Labels
docs Improvements or additions to the documentation external dependency Blocked by external dependency, we can’t do anything about it typescript

Comments

@nihaux
Copy link

nihaux commented Feb 15, 2019

Hello everyone, can't figure out how to solve this so I'm asking for your help :)

Using typescript + react + material-ui (latest versions at the time of writing, cf package.json in repo, link at the bottom).

Can't share the real code but made a small project to reproduce my problem.

I have a generic class like so:

import * as React from 'react';
import { Theme, withStyles } from '@material-ui/core/styles';
import { WithStyles, createStyles, Typography } from '@material-ui/core';

const styles = (theme: Theme) =>
  createStyles({
    typo: {
      width: '100%',
    },
  });

type TestCompWithStyleProps<T> = {
  text: string;
  onClick?: (arg: T) => void;
} & WithStyles<typeof styles>;

export class TestComp<T> extends React.Component<TestCompWithStyleProps<T>> {
  render() {
    const { classes, text } = this.props;
    return <Typography className={classes.typo}>{text}</Typography>;
  }
}

export const TestCompWithStyle = withStyles(styles)(TestComp);

The aim here is to be able to have typings in the onClick callback function.
(kind of like what apollo-client is doing with Query && Mutation components).

It is used like this:

import React, { Component } from 'react';
import logo from './logo.svg';
import './App.css';
import { TestComp, TestCompWithStyle } from './TestComp';

// ok
class TestCompChild extends TestComp<{ someVal: string }> {}

// Type error: Type 'ComponentType<Pick<TestCompWithStyleProps<{}>, "text" | "onClick"> & StyledComponentProps<"typo">>' is not a constructor function type.  TS2507
class TestCompWithStyleChild extends TestCompWithStyle<{ someVal: string }> {}

class App extends Component {
  render() {
    return (
      <div className="App">
        <header className="App-header">
          <img src={logo} className="App-logo" alt="logo" />
          <p>
            Edit <code>src/App.tsx</code> and save to reload.
          </p>
          <a
            className="App-link"
            href="https://reactjs.org"
            target="_blank"
            rel="noopener noreferrer"
          >
            Learn React
          </a>
        </header>

        <TestCompChild
          classes={{ typo: 'toto' }}
          text="Hello from comp"
          onClick={val => {
            val.someVal; // ok
            val.test; // error type as expected
          }}
        />
      </div>
    );
  }
}

export default App;

As you can see, it works fine when I extend the "original" component (not wrapped withStyles).
I get the typing I need in the onClick call prop.
But I can't make it work with the wrapped component (with "withStyles").
I get the error in the comment.
Any idea ? thx :)

PS: here is the repo which reproduce my problem, if you want to run it yourself (create-react-app typescript + material-ui https://github.com/nihaux/typescript-generic-material-ui-withstyle)

@eps1lon
Copy link
Member

eps1lon commented Feb 15, 2019

I have a generic class like so:

That is unfortunately not in our power. Every higher-order component I know so far looses the generic type information.

It's hard to explain but HOCs usually take a generic component Component<Props> and return a different component with some type operation on P Component<Map<Props>>. What you're basically asking is take a Component<Props<T>> and return Component<Map<Props<T>>>. Where does typescript get the type of T from? What if the props are not generic or take more than one type argument?

The only workaround is to override the exported type somehow so that you end up with

export class MyGenericComponent<T> extends React.Component<MyOriginalComponentProps<T> & StyledComponentProps<typeof styles>> {};

I'm very interested in how other higher-order components would approach this issue.

@HenriBeck
Copy link

We have the same issue on react-jss. I don't think there is a solution for this; even overloading doesn't work, because you can't have a type in generics which also has generics as far as I know.

The only "real" solution is to have another component around the withStyles higher-order-component (Basically a container/presentational component).

@eps1lon eps1lon added external dependency Blocked by external dependency, we can’t do anything about it docs Improvements or additions to the documentation labels Mar 12, 2019
@eps1lon
Copy link
Member

eps1lon commented Mar 12, 2019

I'm happy to improve the typings if somebody knows any higher-order component that doesn't loose generic type information. Until then I fear this is not actionable for us since it's a limitation of typescript.

Leaving this open since we should mention this somewhere in our docs. Either link to another resource that describes generic component and hocs or write it ourself. Looking for contributions.

@jeremy-coleman
Copy link

jeremy-coleman commented Mar 13, 2019

i use something similar to this for typestyle / csx / mergeStyles and works great. styled components will be restricted to only proper dom props and the additional props you pass in as types

You have to use a simple css function though, which I don't know where that lives in the mui library.
ie: dont return a stylesheet, just a single rule

import React, { HTMLProps } from 'react';
import clsx from 'clsx';
import {hoistStatics} from './hoist';
import {withStyles, WithStyles, makeStyles} from '@material-ui/styles';
import { CSSProperties, Styles } from '@material-ui/styles/withStyles';
const h = React.createElement

type DomProps = Partial<HTMLProps<any>>
type CSX = { css?: CSSProperties , clone?: boolean, children?: React.DetailedHTMLProps<any, any>}
type StyledCSX = DomProps & CSX & Partial<WithStyles<any>>
//type StyledCSX = DomProps & CSX & Partial<WithStyles<any>> & Partial<Styles<any, P>>
type Proptional<P> = Styles<any, P> | ((...args: P[]) => Styles<any, P>)

export function typestyled<P>(C) {
  return (...props: Proptional<P>[]) => {
    const Comp = (originalProps: P & DomProps) => {
      const className = [
        originalProps.className,
        ...props.map(a => typeof a === 'function' ? a(originalProps) : a)
        .filter(s => !!s)
        .map(s => withStyles<Styles<any, P>, any>(s)) //<-- not gonna work
        //.map(s => cssFunction(s)) //<-- replace with something like this, where css function creates only 1 css rule, not stylesheet-like
        //style(originalProps.classes || {}), <--pick up original classes here
      ].join(' ').trim() //u can use clsx here if u want i guess , but dont need to

      return h(C, ({className, ...originalProps, props}))
    }
    return Comp
  }
}
 return (...props: Proptional<P>[]) => {

is replacing the current mui impl of

const componentCreator = (style, options?) => {

and the biggest thing is applying the styles before you return, not returning the withStyles function like you are currently. I tried using makeStyles and doing like an inline hoc type thing but i couldn't get it work :\ that might be a better option though

https://github.com/jeremy-coleman/jss/tree/master/src

I also was trying to get a working version of jss in typescript up and going and asked for help from jss team but nothing so far - maybe you guys will find it helpful. I think some fixes need to be made, which i mentioned here cssinjs/jss#1053

the stylesheet is the root of the problem - notice the constructor and public 'options' property have different types. also the options.Renderer(this) -- the Renderer there is referencing a type - i don't know how that even works in the first place
https://github.com/jeremy-coleman/jss/blob/master/src/jss/StyleSheet.ts

@eps1lon
Copy link
Member

eps1lon commented Aug 28, 2019

DefinitelyTyped/DefinitelyTyped#37087 addresses the same issue: Generic types are lost with higher-order functions.

See microsoft/TypeScript#30650 (comment) for the current conclusion of this issue.

Closing this since it's not actionable for us and I'd much rather document the issue in the react-typescript-cheatsheet.

@eps1lon eps1lon closed this as completed Aug 28, 2019
@seansfkelley
Copy link

I haven't come up with a good pattern for the general case (HOCs losing generics), but after some exploration I came up with this workaround that strikes an acceptable (for me) balance of readability, simplicity, flexibility and repetitiveness. It's not ideal, but it's little enough work that I'm willing to take up brainspace for it in exchange for the safety it provides.

export type GenericWithStyles<T extends WithStyles<any, any>> = 
  Omit<T, "theme" | "classes"> &
  { classes?: T["classes"]; } &
  ("theme" extends keyof T ? { theme?: Theme } : {});

Usage:

const styles = (theme: Theme) => createStyles({ ... });

interface Props<T> extends WithStyles<typeof styles, true> {
  // ...
}

const MyComponent = withStyles(styles, { withTheme: true })(MyComponentStyled) as 
    <T>(props: GenericWithStyles<Props<T>>) => React.ReactElement;

Note that it requires a function declaration at each usage: you need to provide a type parameter declaration that Typescript can infer/allow you to specify. If you tried to do something like ... as GenericWithStylesWrapper<T>;, you'd have to find somewhere to declare T.

@ZiedHf
Copy link

ZiedHf commented Nov 8, 2019

@seansfkelley Works in my case. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation external dependency Blocked by external dependency, we can’t do anything about it typescript
Projects
None yet
Development

No branches or pull requests

7 participants