WIP: Static rendering
This PR implements static rendering of the styleguide when building for production.
🚧🚧🚧 Please don't merge yet. Still WIP!!! 🚧🚧🚧
It looks like I was able to render everything besides the actual components (which is by design) statically using latest commit.
This includes:
- TOC (Table of Contents) with proper slugs (this took some time)
- Markdown documents
- Code highlighted with highlight.js
We'll need to take some care of injecting CSS into head during the build time as well.
@kof can you help me here with rendering styles statically? SG isn't using react-jss so no SheetsRegistry etc.
BTW @sapegin should we switch to react-jss? Can you elaborate in more details what's is missing from it to be able to use the "official" repository?
Codecov Report
Merging #330 into master will decrease coverage by
<.01%. The diff coverage is97.95%.
@@ Coverage Diff @@
## master #330 +/- ##
==========================================
- Coverage 94.77% 94.77% -0.01%
==========================================
Files 75 78 +3
Lines 995 1014 +19
Branches 202 211 +9
==========================================
+ Hits 943 961 +18
- Misses 52 53 +1
| Impacted Files | Coverage Δ | |
|---|---|---|
| scripts/schemas/config.js | 96.96% <ø> (ø) |
:arrow_up: |
| scripts/make-webpack-config.js | 100% <100%> (ø) |
:arrow_up: |
| loaders/utils/getStyleguide.js | 100% <100%> (ø) |
|
| scripts/create-server.js | 90.9% <100%> (ø) |
:arrow_up: |
| ...sg-components/HtmlDocument/HtmlDocumentRenderer.js | 100% <100%> (ø) |
|
| loaders/styleguide-loader.js | 100% <100%> (ø) |
:arrow_up: |
| src/rsg-components/HtmlDocument/HtmlDocument.js | 100% <100%> (ø) |
|
| ...rc/rsg-components/ReactComponent/ReactComponent.js | 100% <100%> (ø) |
:arrow_up: |
| src/rsg-components/Examples/Examples.js | 83.33% <100%> (ø) |
:arrow_up: |
| src/utils/utils.js | 96.92% <85.71%> (-1.44%) |
:arrow_down: |
| ... and 4 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 2f80d73...c82a157. Read the comment docs.
@okonet I am not aware of decision details, it seems like react-jss would be a good fit here. In any case its easy. You can always create a SheetsRegistry and add the sheets to it and then get them all together as a CSS string.
@kof can you commit the solution you're suggesting?
@okonet @kof Basically these two lines. We need to merge styles with user’s styles and and also pass merged theme object. And strip Renderer for nicer class names.
I’m not sure it’s all possible to do with react-jss and what would be the best solution.
Btw. WrappedComponent.name is not supported everywhere, haven't found caniuse data in this regard.
react-jss uses options.meta property if you pass it, otherwise it falls back to a component name https://github.com/cssinjs/react-jss/blob/master/src/index.js#L36
Why do you call them SomethingRenderer anyways and then replace the Renderer part in order to have nicer meta attributes on style tags? Why not to remove the Renderer thing from the name from the beginning? If it is not a container component, it is a renderer anyways, containers have no sheets.
Btw. WrappedComponent.name is not supported everywhere, haven't found caniuse data in this regard.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/name
Why do you call them SomethingRenderer anyways and then replace the Renderer part in order to have nicer meta attributes on style tags?
Because I have components without Renderer but they don’t have any markup.
Btw. latest react-jss also passes classes object to the component additionally to the sheet.
Migration should be trivial.
@kof What about merging with custom styles and passing theme object?
Because I have components without Renderer but they don’t have any markup.
So they are containers? Why is it important to identify a container by name? Why not call a container component SomethingContainer?
Components without Renderer are “containers”:
function Button(props) {
const stuff = do something with props
return <ButtonRenderer {...stuff} />;
}
Renderers contains only HTML so users could easily change style guide rendering.
And of course I don’t want to write ButtonContainer instead of Button everywhere ;-)
@kof What about merging with custom styles and passing theme object?
Why are you using context to pass the config and take the user styles overrides if they are basically global? Why not just merge them in each styles definition?
Because I don’t want users to do that when they override our components.
@sapegin @kof let's move off-topic discussions to separate issues since they will be lost as soon we merge it.
Ok should we make this whole styles discussion a separate thing? I see 2 possibilities: we add SheetsRegistry right now here. Otherwise I need to implement a ThemeProvider in react-jss. It is planned anyways. Right now the first one is faster.
Yes, let's move the discussion and implement the fastest way and refactor later.
Then let’s make fast now and then we’ll se if we can switch to react-jss :shipit:
So should I do it or it is already clear enough?
Please do this @kof! I've tons of other things to do now.
Should it be on top of this branch? feat-static-rendering
I'd say so since it belongs together.
@kof Yup, you should have commit right now. Welcome to the club but don’t talk about it ;-)
Ok I have added the code for sheets aggregation, but I couldn't test it, somebody need to show me how to kick this all off, or maybe it is right now in a broken state?
@kof try yarn run build
Ok, I have finished the SSR integration for JSS. I needed to do 2 suboptimal things, because of "react-html-document" component limitations.
- We need to get the styles used by the application before we render the document. That module assumes we don't. So I needed to render the app to string first and then pass it to HtmlDocument inside of a div with dangerouslySetInnerHTML.
- HtmlDocument abstraction doesn't give any option to define an attribute on server-side rendered style tag, in order to delete that tag once js kicks in I currently remove the first tag, but it will break if you add more style tags on the server and they will come first.