Skip to content
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

[docs] Warn about using withRoot HOC more than one time per page #12692

Merged
merged 3 commits into from
Aug 28, 2018

Conversation

oorestisime
Copy link
Contributor

Based on discussions here #12649 and gatsbyjs/gatsby#2116 i think the gatsby example deserves a small warning on the usage of the withRoot HOC

@oorestisime oorestisime force-pushed the gatsby-example branch 2 times, most recently from e13697d to c3d5ddc Compare August 28, 2018 15:01
## Providing the theme

To propagate the theme to a component tree use the `src/withRoot.js` HOC.
You should wrap **only** your top-level components with this HOC otherwise you risk re-rendering your React tree multiple times and styling issues during the build phase.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"and causing styling issues"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I provided some screenshots on the #12649 . dont know how to describe best the issues in the screenshots.

@oliviertassinari
Copy link
Member

@oorestisime Does it mean we can't use withRoot with more than 1 page?

@oorestisime
Copy link
Contributor Author

@oliviertassinari yes we can. there are two possibilities (i think)

  • use withRoot only once on the layout component of the pages (i.e all pages use the same layout)
  • use withRoot in each page but not at the layout (or any other component) (i.e pages use different layouts like index, 404 etc)

@oliviertassinari
Copy link
Member

@oorestisime Oh ok, so people shouldn't mount two withRoot on the same page? Yes, that makes sens. It's same for all the other examples.

@oorestisime
Copy link
Contributor Author

Yes exactly, and that was the source of the problems i was experiencing. I think all the repos i saw around using material-ui and gatsby were using withRoot with the problematic way i was using it and that's why i thought of clearing this out in the docs. Let me know if i need to rephrase or anything

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Aug 28, 2018
@oliviertassinari
Copy link
Member

@oorestisime Ok thanks. I think that we should be updating the Next.js README.md with the same message. It the same architecture.

@oliviertassinari
Copy link
Member

Oh no, my bad, we have recently updated the Next.js example to use _app.js. So, it's Gatsby only.

@oliviertassinari
Copy link
Member

@mbrookes I have tried a different wording. Feel free to modify it.

@oliviertassinari oliviertassinari changed the title [docs] Improve gatsby example giving more details on the HOC component [docs] Warn about using withRoot HOC more than one time per page Aug 28, 2018
@mbrookes mbrookes merged commit 9fb136f into mui:master Aug 28, 2018
marcelpanse pushed a commit to marcelpanse/material-ui that referenced this pull request Oct 2, 2018
…#12692)

* [docs] Improve gatsby example giving more details on the HOC component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants