cssinjs / jss

JSS is an authoring tool for CSS which uses JavaScript as a host language.
https://cssinjs.org
MIT License
7.08k stars 399 forks source link

Add Documentation for a Migration HOC #1475

Open ITenthusiasm opened 3 years ago

ITenthusiasm commented 3 years ago

What Would You Like to Add/Fix?

This is a PR to add documentation for creating a custom higher order component (HOC) using the new hooks-based API. It includes explanations for both JS and TS users, and it provides some warnings for anyone previously using decorators. This PR satisfies #1459.

As with before, TypeScript tests are included for the documentation's TS example. (This proved useful/necessary, given that it's not quite the same as the deprecated HOC-based API.)

Todo

ITenthusiasm commented 3 years ago

Added one quick amend. I added a small simplification of the TS types. The less people have to understand/think about, the better.

kof commented 3 years ago

Dunno what to do with this pr @ITenthusiasm , I just merged https://github.com/cssinjs/jss/pull/1508 which rewrites WithStyles to use hooks internally which helped a lot making sure the hoc is doing the same thing as it was before.

Maybe now its time to continue with this plan and go to the next stage of removing WithStyles from the bundle. I will release it hopefully today and we can monitor if that has caused problems, after that we will know this 50LOC can be just safely copy pasted from an example.

ITenthusiasm commented 3 years ago

I've been a bit more preoccupied than I originally imagined as of recent. If withStyles has already been migrated, then I guess we'd just need the example code like you said. I imagine that should be fine. I'd have to examine the new code myself to see what's going on.

I just merged #1508 which rewrites WithStyles to use hooks internally which helped a lot making sure the hoc is doing the same thing as it was before.

That sounds awesome. Quick question (forgive my ignorance), did ReactJSS ever intentionally / explicitly support decorators in v10? I noticed ReactJSS is still v10 on npm. If withStyles was changed to use hooks, I imagine it will cause problems for people using decorators. (It's an easy fix for those people, though.)

kof commented 3 years ago

We have one example with decorators, that has been there since many years and for 2 years we have the HOC syntax in that separate file discouraging using it

We never fully deprecated the HOC though. When you say decorators, you mean I guess actual decorator syntax and I am not sure whats the state of the modern decorators, I think they don't work any more the way stage 2 proposal was, but if they do, they most likely compile to the same hoc one would write by hand

ITenthusiasm commented 3 years ago

When you say decorators, you mean I guess actual decorator syntax and I am not sure whats the state of the modern decorators

Yes. I'm not fully certain what the state of modern decorators is either. :thinking: But basically, from what I recall, class-based HOCs in React are inherently structured in a way that makes them usable as decorators. I found this out at my old job a few months back.

When testing some of these changes on that project, I discovered that HOCs that used hooks can't be used as decorators for class components.

If decorators are already discouraged, it probably isn't a huge deal. Just something to be aware of.