-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[Suggestion] Consider RenderProps instead of HOCs for WithStyles #8542
Comments
@vyrotek I thought this pattern was called compound components 🤔 .
I'm closing, feel free to argue with any of the points I have already be raising against it :). |
You might be interested in this issue #8447. It's a tradeoff. |
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 #7636 (comment) 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 #7636 (comment) 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. |
To summarize, I think that the issues with the pattern are the following by order of importance:
Of course, I'm focusing on the cons, this has pros too! |
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. |
@vyrotek Thanks for the link, I will read it.
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 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 Personally, I'm using I'm gonna ping @kof as the topic can be extended to |
@vyrotek Let me know what you pick, all the feedbacks are welcomed! @sebald Given the TypeScript definition of |
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. |
What benefit exactly would it be for styling? |
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?The text was updated successfully, but these errors were encountered: