SSR: generated class names are not consistent for client and server bundles since v5.1.3

[OliverJAsh]

Bug report

We are using css-loader with server-side rendering (SSR), so we run webpack twice: once for the client and once for the server.

For the client, we need to generate CSS files, so we use mini-css-extract-plugin in combination with css-loader.

For the server, we don't need to generate CSS files, so we use exportOnlyLocals: true. We just need the CSS modules to export the same class names as the ones used in the CSS files we generated for the client (above).

Actual Behavior

Generated class names are not consistent for client and server bundles.

Expected Behavior

Generated class names are consistent for client and server bundles.

How Do We Reproduce?

Full reduced test case: https://github.com/OliverJAsh/webpack-css-loader-ident-hash-bug

package.json:

{
  "dependencies": {
    "css-loader": "^6.3.0",
    "mini-css-extract-plugin": "^2.4.2",
    "webpack": "^5.58.1",
    "webpack-cli": "^4.9.0"
  }
}

webpack.config.js:

const MiniCssExtractPlugin = require("mini-css-extract-plugin");

const { MODE } = process.env;

module.exports = {
    plugins: [new MiniCssExtractPlugin()],
    mode: "production",
    entry: "./src/main.js",
    module: {
        rules: [
            {
                test: /\.css$/,
                exclude: /node_modules/,
                use: [
                    ...(MODE === "client" ? [MiniCssExtractPlugin.loader] : []),
                    {
                        loader: "css-loader",
                        options: {
                            modules: {
                                exportOnlyLocals: MODE === "server",
                            },
                        },
                    },
                ],
            },
        ],
    },
};

src/main.js:

import styles from './main.css'
console.log(styles.container);

src/main.css:

.container {
    display: block;
}

Since this change which first appeared in v5.1.3, css-loader generates different class names for the client/server:

$ rm -rf dist && MODE=client webpack && node dist/main.js
UgjPTt4sSFtvFL9kN_LV

$ rm -rf dist && MODE=server webpack && node dist/main.js
BZjOBwj614XLb60af45_

It seems this is because css-loader now uses relativeMatchResource as part of the hash, and in this case, relativeMatchResource will be different depending on whether or not we're using mini-css-extract-plugin:

• MODE=server (mini-css-extract-plugin is used): relativeMatchResource is ''
• MODE=client (mini-css-extract-plugin is not used): relativeMatchResource is 'src/main.css\x00'

We are able to workaround the issue by downgrading to v5.1.2 which does not include this change.

Please paste the results of npx webpack-cli info here, and mention other relevant information

  System:
    OS: macOS 11.6
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 3.09 GB / 16.00 GB
  Binaries:
    Node: 16.3.0 - ~/.nvm/versions/node/v16.3.0/bin/node
    Yarn: 1.19.1 - /usr/local/bin/yarn
    npm: 7.15.1 - ~/.nvm/versions/node/v16.3.0/bin/npm
  Browsers:
    Chrome: 94.0.4606.71
    Chrome Canary: 96.0.4664.2
    Firefox: 90.0.2
    Firefox Nightly: 77.0a1
    Safari: 15.0
    Safari Technology Preview: 15.4
  Packages:
    css-loader: ^6.3.0 => 6.3.0
    webpack: ^5.58.1 => 5.58.1
    webpack-cli: ^4.9.0 => 4.9.0

[alexander-akait]

hm, weird, I will look at this in near future

[inoyakaigor]

Also got that error. Downgrade to Node 14 did not solve the problem

[OliverJAsh]

After upgrading to 6.4.0 which contains the fix from #1385, I'm getting this error in my app:

[webpack-cli] HookWebpackError: Circular hash dependency 54047 (runtime.54047.js) -> dff12 (education-route.dff12.js) -> 54047 (runtime.54047.js)
    at makeWebpackError (/Users/oliverash/Development/unsplash-web/node_modules/webpack/lib/HookWebpackError.js:48:9)
    at /Users/oliverash/Development/unsplash-web/node_modules/webpack/lib/Compilation.js:2974:12
    at eval (eval at create (/Users/oliverash/Development/unsplash-web/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:25:1)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
-- inner error --
Error: Circular hash dependency 54047 (runtime.54047.js) -> dff12 (education-route.dff12.js) -> 54047 (runtime.54047.js)
    at add (/Users/oliverash/Development/unsplash-web/node_modules/webpack/lib/optimize/RealContentHashPlugin.js:263:16)
    at add (/Users/oliverash/Development/unsplash-web/node_modules/webpack/lib/optimize/RealContentHashPlugin.js:270:9)
    at /Users/oliverash/Development/unsplash-web/node_modules/webpack/lib/optimize/RealContentHashPlugin.js:276:7
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
caused by plugins in Compilation.hooks.processAssets
Error: Circular hash dependency 54047 (runtime.54047.js) -> dff12 (education-route.dff12.js) -> 54047 (runtime.54047.js)
    at add (/Users/oliverash/Development/unsplash-web/node_modules/webpack/lib/optimize/RealContentHashPlugin.js:263:16)
    at add (/Users/oliverash/Development/unsplash-web/node_modules/webpack/lib/optimize/RealContentHashPlugin.js:270:9)
    at /Users/oliverash/Development/unsplash-web/node_modules/webpack/lib/optimize/RealContentHashPlugin.js:276:7
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

I suspect this is a regression from the other change in that release: #1382

Update: never mind, fixed by changing contenthash:5 to contenthash.

[alexander-akait]

Yes, try to avoid uses very small value for hashes, we have ideas how to improve this for webpack v6, but now we have some limitations with very small value of contenthash

[jsg2021]

@alexander-akait After updating to 6.4.0, I'm getting classname collisions. I'm trying to figure out how the two change in the release would do that, but reverting to 6.3.0 fixes them.

[alexander-akait]

@jsg2021 hm, can you try to revert these changes locally c7db752?

[alexander-akait]

And can you provide example of collusion, i.e. class/filename/context?

[jsg2021]

I'll test that.

Here is an example of collision:

import { styled } from 'astroturf/react';
import { User } from '../commons';

export const Container = styled.div`
	cursor: pointer;
	width: 70px;
	height: 70px;
	display: flex;
	align-items: center;
	justify-content: center;

	img,
	svg {
		width: 100%;
	}

	&:global(.flyout-open) {
		background-color: white;
		box-shadow: -1px 0 0 0 #dcdcdc;
		transition: background-color 0.3s, box-shadow 0.3s;
	}
`;

export const Box = styled.div`
	position: relative;
	width: 42px;
	height: 42px;
`;

export const Dot = styled(User.Presence)`
	position: absolute;
	right: 2px;
	bottom: 2px;
`;

on 6.4.0, each of these has the same class generated. Astrotruf extracts each template string into a virtual ComponentName.module.css with a classname based on the component's name in code. They should all get a unique class name each... except they're not. My localIdentName config is the recommended one:

localIdentName: PROD
	? '[hash:base64]' 
	: '[path][name]__[local]--[hash:base64]',

[jsg2021]

Reverting c7db752 didn't fix it, but reverting 303a3a1 did.

Would restoring the relativeMatchResource bits break the consistency SSR part?

[alexander-akait]

@jsg2021 Because we can't rely on matchResource, for example when we extract something it can change module or other loader above can change context (astroturf do it, i.e. we don't know resourcePath), also we can't change something on context above, loader above should provide context for us, ideally astroturf should calculate hash for each js file (from the place where astroturf do extract), based on resourcePath and provide it to css-loader.

As you can see the original post in this issue about this. mini-css-extract-plugin create pseudo module for extracting and previously we rely matchResource, so we got different locales.

[jquense]

i'm up to do whatever but what is the suggestion here? How would astroturf provide css-loader a hash? I'd love to not use matchResource at all, but that was the path we were led down for doing virtual modules.

[alexander-akait]

@jquense We need pass to css-loader !!css-loader!./index.css?modules-hash-salt=hash (pseudo example), is it possible?

[jquense]

ah maybe, that should be fine to provide a hash in the request query...will style and extract loader pass that through when they rewrite the request?

That seems a lot better actually. css-loader would use the hash for calculating the class name hashes?

[alexander-akait]

will style and extract loader pass that through when they rewrite the request?

Should use, if we have !!another-loader!css-loader?modules-hash-salt=hash!another-loader!./index.css, it will work

css-loader would use the hash for calculating the class name hashes?

Yes, we need small changes here, but it is not hard to implement

[jquense]

adding the query to the css-loader itself is a bit harder, we'd have to rewrite the current loader chain with the new option...i think i've done that in the past tho, just a bit messy

[alexander-akait]

@jquense Does astroturf have pitch phase?

[alexander-akait]

The main problem what virtual module doesn't have resourcePath, which used for locals generation and there are two possible fixed:

1. Allow to pass hash for css-loader (i.e. you can emulate different files)
2. Added API to loaderContent for add/renaming current resourcePath, so you can create virtual paths, like src/foo/bar/index.js.css for extracted content and css-loader will use it for generation locals

[jquense]

you'd have the best idea what works. It's a bit frustrating we couldn't have a resourcePath supported out of the box but webpack for these, but i'm sure there is a good reason

[alexander-akait]

Let's keep open until we solve it

[jquense]

@alexander-akait who needs to move first?

[alexander-akait]

@jquense we discussed with @sokra, we will implement this in near future

[jquense]

ok i'll hang and wait for that then 👍 should i tell users to not upgrade to the latest css-loader until then?

[alexander-akait]

Yes, better ask to keep old version

[jquense]

ok let me know if there is anything i can do to help. it is a bit tough to have people pin a minor version of css-loader, esp in frameworks.

[evnp]

Wondering if any progress is in sight here? We're currently stuck pinning css-loader 6.3.0 within another dependency due to ^6.4.0 incompatibility with astroturf (which is very annoying with our current version of npm – need to upgrade).

[jquense]

I would also love to not pin css-loader anymore, is there anything i can do here to move this along? I don't think this is just an astroturf issue, anything that uses matchResources (which is now documented on the doc site even) will have a problem here

[alexander-akait]

@jquense Sorry for delay, I lose context, pinned issue to return to this in near future, sorry again

[jquense]

For quick refresh:

when css-loader hashes classnames, it only looks are resourcePath. which is "wrong" for resources imported with !=! match resource, resulting in duplicate classNames for:

'style1.css!=!button.js'
'style2.css!=!button.js'

[alexander-akait]

@jquense Yeah, I see, looking at both side will be wrong too, because sometimes you want the reversed behaviour, why don't use ?modules=true in this case? I can improve logic and we will accept this

Will ?hash help here? I can implement this

[alexander-akait]

@jquense I think I found the main problem, can you try #1480, looks like I should not concatenate resource path with resource match, instead I should replace 😕

[jquense]

@alexander-akait that PR seems to work! do you think that's sufficient? something like ?hash would also work but really only if it was on the resourceQuery, not inline loader config, Otherwise we need to rewrite the whole loader chain which i've had trouble with in the past and the benefit of the inline match syntax is that i don't have to know what loaders match it

[alexander-akait]

@jquense

do you think that's sufficient?

I think yes, it was my fault, we should not concat such things, I will do release, because no problems with SSR and allow to solve your problem