Generate the same class names for server and client if source maps are different

[mgroenhoff]

What: Generate the hash for the class name from the generated CSS without the source maps

Why: A different babel setup for server and client can change the way how source maps are generated (like the sourceRoot property or different paths to source files) but the resulting styles are the same.

How: Do not include the source maps when creating the hash used for the class name

Resolves #688.

[codecov]

Codecov Report

Merging #695 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted Files	Coverage Δ
packages/create-emotion/src/index.js	100% <100%> (ø)	⬆️

[Andarist]

Could u describe in more detail why ur babel setup is different for client & server?

[mgroenhoff]

For my server-side code I run typescript and then babel. For my client-side code I run typescript and then i run webpack with the babel-loader. I run typescript once for both my client and server code in a single run. However i run babel twice, once for my server code (with the node target) and once, indirectly via webpack for my client side code (with the browser target) and but they output code in different directories. The emotion babel plugin that generates the source maps generates different paths (because they come from a different place). However the CSS has not changes because it comes from the same source.

Lets say the emotion babel plugin adds a source map to some piece of CSS. The decoded source map for the client will look something like:

{
  version: 3,
  sources: [ 'path/to/client/code.jsx' ], // different
  names: [],
  mappings: [ '...' ], // this is identical
  file: 'path/to/client/code.jsx', // different
  sourceRoot: 'path/to/workspace/root',
  sourcesContent: [ '...' ], // this is identical
}

And this will be the equivalent on the server:

{
  version: 3,
  sources: [ 'path/to/server/code.jsx' ], // different
  names: [],
  mappings: [ '...' ], // this is identical
  file: 'path/to/server/code.jsx', // different
  // for some reason it does not include a sourceRoot property
  sourcesContent: [ '...' ], // this is identical
}

[Andarist]

Does this problem affect production env only? I suspect it does not and any solution for this should be applied regardless of the environment

[Andarist]

Oh, after investigating the source code some more it seems that source maps are handled only in non-production code anyway, so this check here is OK. I would hoist sourceMapRegEx up and reuse it at both places.

[mgroenhoff]

One has the global flag and the other has not. I could however do something like this:

const sourceMapRegEx = /\/\*#\ssourceMappingURL=data:application\/json;\S+\s+\*\//
const sourceMapRegExGlobal = new RegExp(sourceMapRegEx, 'g')

Or do you have another suggestion?

[Andarist]

if they are not identical then just leave them as is for now

PS. TIL that RegExp's instance can be used as parameter to RegExp's constructor 👍 always have used it only with string literals 😉

[emmatown]

Could you explain more about your use case? We don't support source maps in SSR so I don't understand why this would happen.

[mgroenhoff]

The issue is not that I want to have source maps server-side, it is that if I want them client-side, I need the server-side to generate the same class names. The class name hash is generated based on the CSS which now includes the base64 encoded inline source map. I need it to create the class name hash only from the actual CSS so that when source maps are different or absent server-side so that React does not error about not being able to hydrate because the class name attribute differs.

[mgroenhoff]

Wrong comment button...

[emmatown]

Okay, makes sense. This change means we're checking the string for the source map twice, could you change it so this check also sets the source map variable and remove the other one?

Also, checking process.env is expensiveish in node so it's best to do the checks at initialization time(though don't define a isDev variable or something since that means the code won't be removed in production on the web). So could you change it so it's something like the production checks in index.js and utils.js in create-emotion?

[fforres]

Currently we are having this same issue on SSR our CMS, so really excited by this PR.
Awesome @mgroenhoff !! :D

[emmatown]

LGTM in terms of code, could you merge from master and fix the merge conflicts?

[Andarist]

I've done the merging, once CI gets green gonna merge it.

[mgroenhoff]

Thanks guys!