New old id generation algorithm - back to the future #432

kof · 2017-02-26T23:19:17Z

Me and @iamstarkov just had a discussion about #356 and decided to reimplement the id generation algorithm.

Right know we JSON.stringify the rules and generate hashes with murmurhash in order to have predictable unique class names in order to support SSR.

We found a way to avoid that overhead and still support SSR.

We revert the algorithm to the very first version where simple counters have been used to ensure the uniqueness.
In order to support SSR, JSS will generate a "rulesmap", which will be used then at runtime on the client in order to obtain the original ids, generated on the server.

This will not only allow us to implement unique class names for dynamic rules in #356 but will also increase an overall performance by factor 3.

cc @nathanmarks @cvle @iamstarkov @typical000 @sapegin @oliviertassinari

kof · 2017-02-26T23:30:33Z

Implementation details:

Id's are simple counters of format: {jssId}-{ruleId}. We need jssId to support multiple jss instances. Both jssId and ruleId are simple counters.
As we need to transfer the rules map to the client, we need some place where it can be placed inside of a document. I see following:
1. We can to tell user to put rulesmap into data-rulesmap attribute of the style tag, define an id on this style tag, tell jss that id.
2. We can let jss generate the style tag with the id and data-rulesmap attribute and return it to the user
3. We can let user use whatever method he wants and let him pass the rulesmap to .rehydrate method.
When jss kicks in on the client, it needs to parse the json string from the style tag attribute, use the ids when creating Rule instances, so that they match exactly the SSR CSS.

oliviertassinari · 2017-02-27T08:09:27Z

That sounds good to me. Does it mean we would no longer have to removed the SSR generated CSS on the client?
Also, that new generation algorithm is allowing really short class names. That's better for the payload.

kof · 2017-02-27T09:54:34Z

Does it mean we would no longer have to removed the SSR generated CSS

We still need to remove, because we have dynamic values, its not not immutable. This should be out of concern though, because its blazingly fast.

nathanmarks · 2017-03-02T00:47:03Z

This is an awesome find/idea. When is it being implemented?

Funny how solutions can stare you right in the face sometimes.

kof · 2017-03-02T08:38:41Z

Its wip, I have been struggling with dynamic rules id generation and @iamstarkov came up with the idea to provide the map from the server and use non-deterministic ids.

kof · 2017-03-02T12:09:33Z

Right now I have still the problem to identify the sheets, I am not sure we can rely on the order we have on the backend and client.

iamstarkov · 2017-03-02T14:08:03Z

@kof what do you mean by "order we have on the backend and client"?

kof · 2017-03-02T15:21:18Z

The order in which we render sheets during ssr and then on the client.

iamstarkov · 2017-03-03T10:28:59Z

we discussed that, and you may have control on that

iamstarkov · 2017-03-03T10:29:04Z

give me a sec

iamstarkov · 2017-03-03T10:38:27Z

my proposed change to jss's ssr https://github.com/cssinjs/examples/blob/gh-pages/react-ssr/src/server.js

import React from 'react'
import {renderToString} from 'react-dom/server'
import {SheetsRegistryProvider, SheetsRegistry} from 'react-jss'
import Button from './Button'

export default function render() {
  const sheets = new SheetsRegistry()

  const app = renderToString(
    <SheetsRegistryProvider registry={sheets}>
      <Button />
    </SheetsRegistryProvider>
  )

  return '' +
`<!DOCTYPE html>
  <html>
  <head>
    <meta charset="utf-8">
    <title>Server-side rendering with rehydration</title>
    <link rel="stylesheet" href="../../example.css" />
-    <style type="text/css" id="server-side-styles">
-      ${sheets.toString()}
-    </style>
+    ${sheets.getStyleTags()}
+    <!-- or -->
+    ${sheets.getStyleTags().forEach(elem => elem.setAttribute('arbitrary', 'attr')) }
  </head>
  <body>
    <a href="https://github.com/cssinjs/examples/tree/gh-pages/react-ssr" title="View on Github" class="github-fork-ribbon" target="_blank">View on Github</a>
    <div id="app">${app}</div>
    <script src="./app.js"></script>
  </body>
</html>`
}

kof · 2017-03-03T10:41:05Z

this isn't much of a problem, the prob is that in pure jss without react-jss we have no idea if sheets are going to be rendered in the browser in the same order as on the server, rehydration data has to rely on that. Even with react I am not sure this is guaranteed.

kof · 2017-03-03T10:43:39Z

Does react guarantees that when rendering on the client first time, it renders exactly the same thing like the SSR did?

iamstarkov · 2017-03-03T10:50:46Z

Does react guarantees that when rendering on the client first time, it renders exactly the same thing like the SSR did?

why wouldnt it?

kof · 2017-03-03T10:51:10Z

dunno, its a question)

kof · 2017-03-03T10:52:58Z

but actually any framework would need to guarantee that, otherwise it would need a full rerender on startup.

kof · 2017-03-03T10:53:53Z

I mean the whole point of rehydration is to start the js powered application on top of server rendered one without rerender the whole thing, if it rerenders, we have no problems anyways.

iamstarkov · 2017-03-03T10:54:37Z

and why do you care about the order?

if styletags would be generated 1 to 1 with components, you need only data-jss-map attribute on styletags wo/ need to care about the order, right?

kof · 2017-03-03T10:56:01Z

if components are the same sheets will be too, but if its possible that the client for e.g. skips rendering one component in the middle, without full rerender, then it is fucked up.

iamstarkov · 2017-03-03T10:57:09Z

i dont understand you

kof · 2017-03-03T11:00:47Z

If server renders this:

SheetA
SheetB
SheetC
CompA
CompB
CompC

But the client renders this:

SheetA
SheetC
CompA
CompC

Without a full rerender of the App. Rehydration will use Data from the server and will use the ids map for SheetC from SheetB, but I guess this shouldn't be possible anyways.

iamstarkov · 2017-03-03T11:11:06Z

there are several cases of this behavior, e.g. when you want
a) decrease ssr cpu load
b) cache components on ssr, but you dont want to cache privacy related components, so on ssr you render layout skeleton to deliver best perceived performance and later on client side you render full-blown app

kof · 2017-03-03T11:12:12Z

In this case, react would rerender the whole thing, right? Rerender in the sense of all components render methods and updating dom if needed.

iamstarkov · 2017-03-03T11:15:05Z

let me check with colleagues

iamstarkov · 2017-03-03T11:16:25Z

but i still cant get why does jss need to care about the order, when you can link stylesheets to components with dom node attributes.

am i right that rulesmaps are scoped to stylesheet, right?

kof · 2017-03-03T11:18:45Z

      sheets.add(jss.createStyleSheet({
        a: {color: 'red'},
        b: {color: 'green'}
      }).attach())
      sheets.add(jss.createStyleSheet({
        c: {color: 'blue'}
      }).attach())
      expect(sheets.toMap()).to.eql([{a: 'a-id', b: 'b-id'}, {c: 'c-id'}])

You can see how the map looks like, its an array, where each object is a map of rule name to a class name and represents a sheet. The order needs to be the same.

kof · 2017-03-03T13:00:29Z

We can advise users to use a function to generate the styles object or to deepClone the styles object when passing it to JSS, because it will mutate that object. Basically users should care about it only if they create multiple sheets using the same styles object.

kof · 2017-03-03T13:02:03Z

Also we could accept a function instead of styles object which returns the styles object in order to make it even a bit simpler

iamstarkov · 2017-03-03T13:31:48Z

      sheets.add(jss.createStyleSheet({
        a: {color: 'red'},
        b: {color: 'green'}
      }).attach())
      sheets.add(jss.createStyleSheet({
        c: {color: 'blue'}
      }).attach())
      expect(sheets.toMap()).to.eql([{a: 'a-id', b: 'b-id'}, {c: 'c-id'}])

we should find a way to identify sheets by some kind of identifier, order is fragile

kof · 2017-03-03T13:33:03Z

we should find a way to identify sheets by some kind of identifier, order is fragile

the only way is to create hashes … are u sure order is fragile?

iamstarkov · 2017-03-03T13:41:29Z

look

sheets.add(jss.createStyleSheet({
  a: { color: 'red' },
  b: { color: () => 'green' }
}).attach())
sheets.add(jss.createStyleSheet({
  a: { color: () => 'blue' }
}).attach())
expect(sheets.toMap()).to.eql([
  { a: 'a-id',
    b: 'b-id' },
  { a: 'a-id' }
])

order is fragile in case if you cant guarantee original rule names

iamstarkov · 2017-03-03T13:44:33Z

sorry

kof · 2017-03-03T14:04:25Z

order is fragile in case if you cant guarantee original rule names

rule names are the same, its about the sheets order, I am not sure its a prob.

cvle · 2017-03-04T07:49:09Z

React Stack definitely has a deterministic order. I assume that the initial render on React Fiber will be deterministic (need verification).

Another note is that Fiber apparently doesn't come with SSR initially and will probably rely on community solutions. I can't say how those community solutions will be. My guess is that it will match the render order on the client (need verification).

Initial version of Fiber won't ship with an SSR renderer but should be easier to build on top
https://twitter.com/dan_abramov/status/792324589594566656

kof · 2017-03-04T09:17:22Z

Can we go with this logic: if SSR renders only critical markup/css - don't call jss.rehydrate(), if client renders identical view when it kicks in - rehydarate?

cvle · 2017-03-04T10:25:19Z

React gives a warning when markup from server differs from the client, can't we assume they are always the same? Who uses server rendering to render a critical markup that differs from the clients render?

cvle · 2017-03-04T10:26:44Z

And another thought I have is: Do we need to transmit a map of class names to the client? Can't the server and client calculate it independently using counters and still have the same result?

kof · 2017-03-04T11:25:07Z

I am wondering right now as well, counters are invoked in exact same way, there is no randomness. If ssr logic is the same, there should be no difference, if it is not, client would render a different view and in this case update the entire document where needed, which essentially mens there is no need in rehydration in such a case.

kof · 2017-03-10T18:30:57Z

Released without rehydration. Lets see if it broke anything.

kof · 2017-03-10T18:31:29Z

Increased only minor version because I expect no breaking change.

rasentry · 2017-04-12T12:15:52Z

@kof This changes broke my SSR.

(client) ader-wrapper fade-0-256 fade-0-191 in-0-
(server) ader-wrapper fade-0-3 fade-0-0 in-0-1" d

kof · 2017-04-12T12:17:03Z

@rasentry do you know how?

rasentry · 2017-04-12T12:19:36Z

@kof I just upgraded to v7 and stil do not see obvious reason why this happen. Trying to understand...

kof · 2017-04-12T12:21:00Z

Let me know if I can help to debug this. Ideally I need some simple working example that reproduces the issue.

jcheroske · 2017-05-10T05:42:06Z

I'm seeing this in next.js as well:

Warning: React attempted to reuse markup in a container but the checksum was invalid. This 
generally means that you are using server rendering and the markup generated on the server was
not what the client was expecting. React injected new markup to compensate which works but you
have lost many of the benefits of server rendering. Instead, figure out why the markup being 
generated is different on the client or server:
 (client) <div class="paper-0-0" style="color:rgba
 (server) <div class="paper-0-2" style="color:rgba

When the next.js server first boots, the jss counter is at 0. If I hit my page everything works, and there is no React warning. If I refresh the browser, for some reason next.js reruns my component file, and re-injects the sheet, which increments the server-side jss counter (This only happens one time). But the browser got refreshed, so its counter is back to zero. Additionally, every time the dev server detects a file change it reruns all the components, and the counter gets incremented yet again.

I think you guys are right in thinking the the load order of React components is deterministic. What might not be though is the counter's starting point on the server. SSR is a weird beast. If you have five sheets, the counter will always increment five times in the same sheet order. But, where it always runs 0, 1, 2, 3, 4 on the client, it might run 13,14,15,16,17 on the server.

And, just to make sure I'm not missing something, is there a special format for the server-generated <style> tag? Meta info of some kind? Reading this thread, I didn't think so.

kof · 2017-05-10T06:44:10Z

So SSR rerenders, but counter continues to increase.

kof · 2017-05-10T06:45:05Z

I think we don't reset the counter between the requests.

kof · 2017-05-10T06:46:53Z

And, just to make sure I'm not missing something, is there a special format for the server-generated <style> tag? Meta info of some kind? Reading this thread, I didn't think so.

no, just what it is in the docs.

Please open a separate issue for this story.

jcheroske · 2017-05-10T18:57:41Z

@kof Just to be clear, you want a new bug report on the next.js counter issue?

kof · 2017-05-10T19:00:43Z

I think we can handle it in #457 as it seems to be specific to next.js

kof · 2017-05-10T19:00:59Z

lets create an example over there and see what we need to fix

kof added enhancement No kittens die if we don't do that. important The thing you do when you wake up! task labels Feb 26, 2017

kof mentioned this issue Feb 27, 2017

Expose murmurhash function used for hash creation #418

Closed

iamstarkov closed this as completed Mar 3, 2017

iamstarkov reopened this Mar 3, 2017

kof mentioned this issue Mar 7, 2017

Feedback from @JedWatson #222

Closed

kof closed this as completed Mar 10, 2017

jcheroske mentioned this issue May 10, 2017

Add an example to next.js #457

Closed

New old id generation algorithm - back to the future #432

New old id generation algorithm - back to the future #432

Comments

kof commented Feb 26, 2017 • edited Loading

kof commented Feb 26, 2017 • edited Loading

oliviertassinari commented Feb 27, 2017 • edited Loading

kof commented Feb 27, 2017 • edited Loading

nathanmarks commented Mar 2, 2017

kof commented Mar 2, 2017 • edited Loading

kof commented Mar 2, 2017

iamstarkov commented Mar 2, 2017

kof commented Mar 2, 2017

iamstarkov commented Mar 3, 2017

iamstarkov commented Mar 3, 2017

iamstarkov commented Mar 3, 2017

kof commented Mar 3, 2017 • edited Loading

kof commented Mar 3, 2017

iamstarkov commented Mar 3, 2017

kof commented Mar 3, 2017

kof commented Mar 3, 2017 • edited Loading

kof commented Mar 3, 2017

iamstarkov commented Mar 3, 2017

kof commented Mar 3, 2017

iamstarkov commented Mar 3, 2017

kof commented Mar 3, 2017

iamstarkov commented Mar 3, 2017 • edited Loading

kof commented Mar 3, 2017 • edited Loading

iamstarkov commented Mar 3, 2017

iamstarkov commented Mar 3, 2017

kof commented Mar 3, 2017

kof commented Mar 3, 2017

kof commented Mar 3, 2017

iamstarkov commented Mar 3, 2017

kof commented Mar 3, 2017

iamstarkov commented Mar 3, 2017 • edited Loading

iamstarkov commented Mar 3, 2017

kof commented Mar 3, 2017

cvle commented Mar 4, 2017

kof commented Mar 4, 2017

cvle commented Mar 4, 2017 • edited Loading

cvle commented Mar 4, 2017 • edited Loading

kof commented Mar 4, 2017 • edited Loading

kof commented Mar 10, 2017

kof commented Mar 10, 2017

rasentry commented Apr 12, 2017

kof commented Apr 12, 2017

rasentry commented Apr 12, 2017

kof commented Apr 12, 2017 • edited Loading

jcheroske commented May 10, 2017 • edited Loading

kof commented May 10, 2017

kof commented May 10, 2017

kof commented May 10, 2017

jcheroske commented May 10, 2017

kof commented May 10, 2017

kof commented May 10, 2017

kof commented Feb 26, 2017 •

edited

Loading

kof commented Feb 26, 2017 •

edited

Loading

oliviertassinari commented Feb 27, 2017 •

edited

Loading

kof commented Feb 27, 2017 •

edited

Loading

kof commented Mar 2, 2017 •

edited

Loading

kof commented Mar 3, 2017 •

edited

Loading

kof commented Mar 3, 2017 •

edited

Loading

iamstarkov commented Mar 3, 2017 •

edited

Loading

kof commented Mar 3, 2017 •

edited

Loading

iamstarkov commented Mar 3, 2017 •

edited

Loading

cvle commented Mar 4, 2017 •

edited

Loading

cvle commented Mar 4, 2017 •

edited

Loading

kof commented Mar 4, 2017 •

edited

Loading

kof commented Apr 12, 2017 •

edited

Loading

jcheroske commented May 10, 2017 •

edited

Loading