Jss: [react-jss] No critical css generated on server

[bungu]

__Expected behavior:__

const sheets = new SheetsRegistry();
const html = ReactDOM.renderToString(
  <JssProvider registry={sheets}>
    <App />
  </JssProvider>
);
const css = sheets.toString();

css should not be empty.

__Describe the bug:__
It seems that SheetsRegistry return empty string in toString method because no stylesheet is attached. Example repo: https://github.com/bungu/jss-no-critical-css

It works in v9

__Versions (please complete the following information):__

• JSS: v10.0.0-alpha.13
• React-JSS: v10.0.0-alpha.6

$ npm list jss react-jss
[email protected] ~/sandbox/jss-no-critical-css
└─┬ [email protected]
  └── UNMET PEER DEPENDENCY [email protected]

npm ERR! peer dep missing: [email protected], required by [email protected]

[HenriBeck]

Please update to the latest react-jss alpha version as well.

[dandrei]

Could this be an issue in 9.8.7 as well? Over night, my sheetsRegistry.toString() started returning an empty string in my Next.js app on Linux (but not on Windows). Maybe it's a dependency?

[kof]

Most likely 2 versions are used in parallel, so registration happens in one, but .toString() call with another.

[oliviertassinari]

The fix could be about:

  /**
   * Attach renderable to the render tree.
   */
  attach(): this {
-   if (this.attached || !this.renderer) return this
+   if (this.attached) return this
+   this.attached = true 
+   if (!this.renderer) return this
    this.renderer.attach()
-   this.attached = true
    // Order is important, because we can't use insertRule API if style element is not attached.
    if (!this.deployed) this.deploy()
    return this
  }

Maybe we miss a regression test too for server-side rendering?

[HenriBeck]

Could this be an issue in 9.8.7 as well? Over night, my sheetsRegistry.toString() started returning an empty string in my Next.js app on Linux (but not on Windows). Maybe it's a dependency?

No, unless you explicitly changed some version, SSR shouldn't just break overnight.

[HenriBeck]

This was introduced in the latest alpha. If you need SSR, please downgrade, for now, I will prepare a fix and a regression test for it.

[HenriBeck]

Maybe we miss a regression test too for server-side rendering?

The issue was that we had a Renderer in our SSR test because the tests still run in a browser.

[kof]

Right, I broke this in the last release

[kof]

Looking into it now, the problem is quite simple, SheetsRegistry returns only sheets which have .attached === true when calling .toString() and as I removed VirtualRenderer lately, changed the logic to not setting attached=true when there is no renderer, which is only the case in non-browser environment, so tests just worked as there was a renderer. Tests for SSR need to always pass Renderer: null

[kof]

Fixed, will be released in 10.0.0-alpha.14

[oliviertassinari]

Well done :)