Material-ui: [Suggestion] Consider RenderProps instead of HOCs for WithStyles

[vyrotek]

Recently there has been some buzz around a talk calling out HOCs as the new Mixins and how they are full of problems. As I've been following the WithStyles TypeScript issues (e.g. #8447) I couldn't help feel like maybe the problem isn't a lack of TS features but rather a possibly a problem with the approach.

Here's an excellent write-up and video of what @mjackson calls "Render Props" and how they can do everything an HOC can and also solves several problems with them.

Use a Render Prop!

I highly recommend watching the video as he does an excellent job illustrating how the React community got to where it is now. (FWIW, MJ is also the author of ReactRouter which currently implements this pattern)

What might WithStyles look like using Render Props instead? Could it help provide better type safety by removing the need for the ! hack and provide more clarity as to what props WithStyles is providing?

[oliviertassinari]

Consider RenderProps

@vyrotek I thought this pattern was called compound components 🤔 .
This issue is interesting, thanks for opening it. However, I believe we have already been exploring this path:

• #7988
• https://github.com/callemall/material-ui/issues/7636#issuecomment-320650426

I'm closing, feel free to argue with any of the points I have already be raising against it :).

[oliviertassinari]

Could it help provide better type safety by removing the need for the ! hack and provide more clarity as to what props WithStyles is providing?

You might be interested in this issue #8447. It's a tradeoff.

[vyrotek]

Good info. Thanks. I hadn't heard of the term "Compound Components" but I have heard it referred to as "Function as Children" as this comment did https://github.com/callemall/material-ui/issues/7636#issuecomment-320659315

Yeah, issue #8447 is what finally did it for me. I love TypeScript but I understand the concerns about not wanting to cater to a decorator feature that isn't finished yet. At the same time, I feel like the root problem is having withStyles mix in extra props which fights the whole concept of typing and we could run into prop conflicts.

So ultimately I guess your comment https://github.com/callemall/material-ui/issues/7636#issuecomment-320763492 about "lifecycles > flexibility" ended the discussion. Although I don't fully understand the reasoning and probably disagree I suppose I'm satisfied knowing it had been discussed before.

[oliviertassinari]

To summarize, I think that the issues with the pattern are the following by order of importance:

1. It requires more boilerplate, It's slightly true for nominal use cases, but definitely painful when needed to access the classes in the lifecycles hooks.
2. We shouldn't, as much as possible, sacrifices pattern the language is allowing for typing constraints. It should be the other way around.
3. Performance, it's hard to tell without a benchmark, but given the fact we are creating a new function at each render x the number of components we have. It's definitely a small overhead.

Of course, I'm focusing on the cons, this has pros too!

[vyrotek]

1. Hmm, I'll need to do more research on this.
2. https://cdb.reacttraining.com/react-inline-functions-and-performance-bdff784f5578
3. Just because you can doesn't mean you should. Even without typing I don't like how much it still feels like mixins. Very large projects may bump into prop naming conflicts and source confusion.

Crazy thought. Could WithStyles support both approaches? I'm guessing 1. is the biggest concern?

Sorry for all the poking! I like what you guys are doing here. I'm evaluating several UI frameworks for a large project and the friction of using WithStyles with TypeScript (and justifying any issues to other devs) is my biggest concern.

[oliviertassinari]

@vyrotek Thanks for the link, I will read it.

Crazy thought. Could WithStyles support both approaches?

Definitely, there is no technical limitation for this. It can even be implemented today on userland. Regarding supporting it on the library side. I'm not sure. It depends on the vision we have with the withStyles() Higher-order Component.

If you go through the docs, it says that it's our internal styling helpers that we expose for convenience to our users. It was first designed for the internal components needs. Users are free to use whatever styling solution they want on top of Material-UI. Of course, the direct access to the theme with withStyles() is great, but it can be replaced. For instance, with the withTheme() Higher-order component or styled-components theming.

Personally, I'm using withStyles() exclusively in userland for all the projects I'm working on. I have also the feeling that a large majority of our best testers do the same (based on the feedback I see).

I'm gonna ping @kof as the topic can be extended to react-jss.

[oliviertassinari]

Sorry for all the poking! I like what you guys are doing here. I'm evaluating several UI frameworks for a large project and the friction of using WithStyles with TypeScript (and justifying any issues to other devs) is my biggest concern.

@vyrotek Let me know what you pick, all the feedbacks are welcomed!

@sebald Given the TypeScript definition of withStyles() is going to change significantly with the next beta release for the better, maybe it's time to add a TypeScript guide in the documentation? What do you think? I have added a very simple Flow guide recently.

[kof]

I have an issue for a HOC-less interface since 2016) https://github.com/cssinjs/react-jss/issues/31 I reopen it now, because render functions are hype now)

Not sure though when it makes really sense. Because we got also dynamic values now.

[kof]

What benefit exactly would it be for styling?