johanholmerin / style9

CSS-in-JS compiler inspired by Meta's stylex
MIT License
570 stars 27 forks source link

Basic SSR support #34

Closed dpeek closed 3 years ago

dpeek commented 3 years ago

Basic support for SSR by storing style declarations and providing flush() function

Apologies if this is a quick hack, was finding my way around your code-base, babel, nextjs, styles-jsx so it's possible this isn't the right way :) Hopefully it's a workable starting point, some todos I noted down:

dpeek commented 3 years ago

I note that the tests are failing but only because the generated source is different, tests should pass at runtime 😅

johanholmerin commented 3 years ago

Style9 already has full SSR support, using for example the webpack plugin. Styles are extracted to a single CSS file, that can then inlined or linked to. What issue are you looking to solve?

dpeek commented 3 years ago

My apologies, I should have checked before I went and did this!

I was looking to take the same approach as styled-jsx in next – which collects styles used on server rendered pages to inline into the initial response, with subsequent routes coming down bundled in javascript with Webpack. I suppose the goal is that the client only downloads the styles it needs in the first request, incase your styles grow (extremely) large.

The initial issue I was facing was that using style9 server side (in my custom _document to add some classes to body, for example) were not shipping to the client at all. What would be the best way to resolve that? Should I configure Webpack to compile to a single stylesheet and then include a link to that in _document?

Thanks again for your time.

johanholmerin commented 3 years ago

No worries. Yes, style9 is meant to generate a single CSS file, which because it generates atomic classes, should be sufficiently small. I'm not an expert on next.js, but if you make a repo with your issue, I could take a look at it.

dpeek commented 3 years ago

Basic repro of nextjs issue here: https://github.com/dpeek/style9/tree/next-ssr-issue (I did it as a branch of this repo to make debugging easier)

Thinking about it more, I think the issue here with nextjs (and potentially other frameworks) is that there is no single compilation that allows you to gather all the styles to put in a single stylesheet.

In the repro, there are two seperate compilation targets – server and client – so you would at least need to compile any styles used by the server to a static stylesheet and deliver styles for routes with the JS bundle for each route.

Caveat: also not an expert on nextjs, but I think even the server side for each route might be a seperate compilation, as nextjs is designed to scale to "millions of pages" I think server side routes compiled separately are loaded dynamically. (as seen by the fact you don't get a compile time error until you navigate to a page in dev mode).

If that is true, then perhaps something like this PR is needed after all? I might need to seek the advice of a next expert!

dpeek commented 3 years ago

Any thoughts on the above? Perhaps I am over complicating things...

johanholmerin commented 3 years ago

The issue is that next.js only compiles _document for the server, and therefore doesn't support CSS. You could solve this by defining your styles in another file, that is used by the client as well, and importing the class names into _document.

dpeek commented 3 years ago

That is my solution for the moment, but then you get a flash of unstyled content whilst the javascript loads :) The goal of inlining that css in the html response from the server is that the client gets everything it needs for first contentful paint in one request.

Would be helpful to understand your point of view against adding something like what this PR implements – is it against the philosophy of the library or just unnecessary complexity? Not that it's a good reason, but something similar exists for styled-jsx, styled-components etc.

johanholmerin commented 3 years ago

Here's an overview of how style9 works: At build time, the compiler transforms every source file, removing all style definitions, replacing them with generated class names, and creates a single CSS file for all styles. No style definitions are left in the remaining code, only class names. Later, at run time, all that happens in the code is that the class names get added to the output.

Adding features to register and flush styles serves no point and is not possible: there are no styles in the code at runtime. The next.js plugin could probably be improved(to support creating styles in _document, etc.) by someone familiar with the internals of next.js, but the current version works and I consider wrestling with webpack configs a waste of life. I'm not sure what issue you had with moving the styles, it worked fine for me, and there are no FOUC. If you wish to inline the CSS instead of linking to it, that could be done with a webpack plugin, just like if you used regular CSS files.