Type props/data

[lucasbordeau]

Is your feature request related to a problem? Please describe.

I can't manage to find any way to type the props/data parameter which is currently unknown :

Describe the solution you'd like

It would be nice to provide a type generic parameter on createUseStyle to type data: unknown (in fact I can see in the code that it is left as a TODO)

Are you willing to implement it?

Yes, I'll try to implement it !

[ITenthusiasm]

Any updates on this? I'm hesitant to send a PR because I'm unsure what makes this particularly complicated/difficult.

Looking in createUseStyles.js, I'd imagine another generic type parameter for Props could get added to the createUseStyles function. Then the data parameter for useStyles could be given the correct type.

There are only 4 instances of data in that file. One is for the parameter definition, another is for a React hooks dependency. addDynamicRules and updateDynamicRules from sheets.js are the only 2 functions using data, and they use any for its type. In terms of making sure useStyles enforces receiving the correct argument, the change would be straightforward.

I guess making sure the CSS properties based on props are correctly typed would be potentially more complicated, though.

EDIT: Looking at this now, it looks like these files are using flow. Would the changes for TypeScript to work out smoothly only be in the index.d.ts files?

[ITenthusiasm]

To keep track of things for myself (or whoever ends up implementing this), I'll continue to add what I find. If anyone else has anything to add to the discussion or notices something that's wrong, lmk.

Moving forward with the assumption that the flow files will need to get updated:

Moving down the list, addDynamicRules and updateDynamicRules would need to become generic functions. They would base the generic type on the type of data.

sheet.updateOne seems to be the only function in that file that relies on data. So I guess the rabbit hole only needs to go deeper there.

[ITenthusiasm]

Okay. Pulling back from my rabbit trail. Forgot that in addition to wanting useStyles to expect the right type, it'd also be great for people to get intellisense whenever they're using functions based on props. For the former, I think what I've said would already be enough -- as far as the user is concerned. (Someone can correct me if I'm wrong.)

The larger deal appears to be this. For this, my only question is... Is anything keeping us from making Func accept another generic parameter that's determined from the data argument? From afar it seems like a pretty straightforward change. Are there any repercussions for this?

[kof]

I also think that it should be a generic, but I also haven't tried to add one there to see if there are problems, so I encourage you to start a PR!

[ITenthusiasm]

Okay cool. Similar to what I said for the other issue, I'll try to look into this some time this weekend or next. Again, feel free to ping me if I vanish after the next 2 weeks.

[ITenthusiasm]

EDIT: You can ignore this question. I decided to do a quick test on the ReactJSS playground. The theme is indeed confirmed to be accessible. I learned that theme has to get passed into useStyles, so it makes sense.

@kof Trying to work through the TypeScript path right now for createUseStyles (deciding to focus that instead of the flow JS since that's all TS users will see).

Critical Question:
In the theming portion of the docs, I noticed that theme can be an argument at the top-most level (argument to createUseStyles), or it can be an argument at a CSS Property level (eg. background: ({ theme }) => ...).

However, in the props portion of the docs, it's revealed that props can be an argument at the Class Name level (eg. myClassName: props => ({ ... })), or it can be an argument at a CSS Property level (eg. background: props => ...).

My question is... at the Class Name level, is theme exposed? That is, can someone do this?

const useStyles = createUseStyles(
  {
    myLabel: ({ labelColor, theme }) => ({
      display: 'block',
      color: labelColor,
      fontWeight: theme.fontWeight,
      fontStyle: theme.fontStyle
    })
  }
)

The docs don't seem to suggest that this is possible or recommended. However, if the code enables this, then even if the docs discourage it, it may be wise to make sure the TypeScript types reflect what the code is actually doing. I'm not as familiar with the codebase; do you have insight on this?

[kof]

Released in v10.6.0