suggestion for TS typing of withStyles #8704
Comments
|
Re. using |
|
in this definition though, using the above typings: @withStyles(styles)
export class CompDeco extends React.Components<Props, {}> { }
const CompHoc = withStyles(styles)(CompDeco)
// as far as TS is concerned, typeof CompDeco === typeof CompHocThe above definition is exactly how This definition lets a consumer consume it exactly how they want - strict or loose. |
|
Your example doesn't compile with @withStyles(styles)
export class Comp extends React.Component<Props, {}> {
render() {
return (
<div className={this.props.classes.classA}>Look ma, it works</div>
// ^^^^^^^^^^^^^^^^^^
// Object is possibly 'undefined'.
)
}
}For the typing to be correct, |
This is required by TypeScript decorators in their current form, which is why we can't use them if we want the types to be correct. Because the actual semantics of |
|
ah, I apologise, had my config set wrong. I thought I had @withStyles(styles)
export class Comp extends React.Components<Props, {}> {
render() {
const classes = this.props.classes!
return (
<div className={classes.classA}>Look ma, it works</div>
)
}
}I recognise that it's required by typescript, I raised an issue and spent some time discussing with them (amongst other things) a few weeks ago (because I thought it was a pretty silly way to force the typings, but that's another discussion [along with the way they do generic constructors...]). personally though, I feel like it'd be better to provide a typing for the decorator, rather than no typing for the decorator at all. without a definition, consumers have to create their own wrapper with a decorator typing (like I have), or they HAVE to use it your way. At least with a decorator definition, people can choose to use it if they like, or in the case that it isn't strict enough, they can use the HOC definition. that aside though. also it'd be good to relocate (or re-export) some of the defs so that it can be imported from a single place (as opposed to multiple files which is required right now) |
Yes, this was the compromise I struck in #8354, and prior to
See Typescript #4881. My understanding is they're waiting for decorators to become a stage 3 proposal before they'll reimplement them properly.
I'm not sure what you mean. AFAIK there's no way to "just add support for the decorator" without also making the typings incorrect in one way or another. If we make it usable as a decorator, people can't opt to "use the HOC definition" and get the strictness they want. I'd love to see a proposed typing that proves me wrong on this, though!
You might want to open a separate, more targeted issue (or even a PR!) that disentangles this proposal from the |
I was going to say that function overloads will do what you want, because i've seen and have used them before for these decorator overloads. However upon lengthy testing and investigation, it appears that typescript does its annoying optimisation and just chooses the most generic function definition for you. (another one of their no doubt "helpful" compile optimisations). You can't even hint to the compiler an order in which to attempt them by defining them in a certain order. The best way I found to make it work is as follows: // hoc
declare function fn<P extends WithStylesInputProps>(clazz : React.StatelessComponent<P> | React.ComponentClass<P> | React.ClassicComponentClass<P>) : React.ComponentClass<StyledComponentProps>
// deco
declare function fn<TC>(comp : TC) : TC
const TestHoc = fn(
class testHoc extends React.Component<Props & WithStylesInputProps, {}> {
} as React.ComponentClass<Props & WithStylesInputProps> // note the type cast
)
@fn
class TestDeco extends React.Component<Props & StyledComponentProps, {}> { }Whilst this works and lets you have the dual definitions, it mildly inconveniences those that want the super strict definitions.
I might just do that |
I think “mildly inconveniences” is putting it mildly ;) |
|
We used method overloads when we were allowing decorators; this was the definition: export default function withStyles<Names extends string>(
style: StyleRules<Names> | StyleRulesCallback<Names>,
options?: WithStylesOptions
): {
<P>(
component: React.StatelessComponent<P & WithStyles<Names>>
): React.ComponentType<P & StyledComponentProps<Names>>;
<P, C extends React.ComponentClass<P & StyledComponentProps<Names>>>(
component: C
): C;
};This gave the correct typing when using it with an SFC, and the decorator-friendly-but-slightly-wrong version for class components. Unfortunately there’s just no way of getting around the fact that, from the type system’s point of view withStyles(styles)(class C {})and @withStyles(styles)
class C {}are completely indistinguishable; there’s no way to give an alternate typing for just the situation where you’re using a decorator. |
|
unfortunately it's a bit annoying that they're leaving it in its broken state. function @withStyles(...) // deco def
function withStyles(...) // non deco defI digress though. |
Having a look at your recently updated doc for typescript: https://github.com/callemall/material-ui/blob/v1-beta/docs/src/pages/guides/typescript.md, I wanted to make a suggestion in regards to how to type the decorator.
Within my app I've actually wrapped your function in my own function + typings for two reasons: 1) so that I can import all the relevant interfaces/functions from a single place, and 2) so I can apply decorator typings to the function.
Here is my code for my wrapper:
And usage:
Now I know it might look a little bit weird because there are a few functions that do nothing except for change types, but I found this is the easiest way to provide nice typings around the place.
createStyleSheetexists for a few reasons;withStylesFn's typings are setup so that they work for a decorator, and note that they will work as well as a HOC (though they aren't as strict as some people might like).Note that I don't do any strict typing around the props for the component, this is because at the end of the day - who cares what props are on the component passed in.
If the consumer forgets to put a
classesprop on their component props, their component won't work and they won't be able to access the classes in their render because typescript will throw an error at them.If they don't actually use a
themeprop, why force them to put it on their props? It seems like it's an annoying an confusing thing to do when they don't care and neither does react.This makes it easier to do the typings because the current typings requires the component to have
classesandtheme, then returns a component with them optional.Also important to note how the react typescript typings are written:
And typescript's
Readonlydef:Note that
Readonlyconverts all optional keys to be required (because there's no?:!!!).Which means that we can define the props on the class as
And then within the code we can use it without undefined errors, because of the readonly interface.
For posterity's sake, this is the
jssPropertiesdefinition file.It's just an extension of the css properties with some extra typings to work with the JSS's default plugin set that you add to the JSS config automatically:
The text was updated successfully, but these errors were encountered: