Webpack-based HMR removes style-tag in monorepo setup

[danielberndt]

Describe the bug

Hi there! After upgrading to create-react-app v5 which uses webpack 5.73.0, HMR is breaking after almost every save in our monorepo setup.

I finally was able to create a minimal repro-case here: https://github.com/danielberndt/vanilla-extract-monorepo-issue

I'm attaching a recording of what is happening: When modifying a css file that is used in both its own package and another package within the monorepo, it's style tag gets removed:

Interestingly it doesn't seem to happen if the contents of frontend/src/App.js get inlined into frontend/src/index.js

Let me know if you need any more details!

Reproduction

https://github.com/danielberndt/vanilla-extract-monorepo-issue

System Info

System:
    OS: macOS 11.6.5
    CPU: (6) x64 Intel(R) Core(TM) i5-8500B CPU @ 3.00GHz
    Memory: 277.28 MB / 20.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.0 - /usr/local/bin/node
    Yarn: 1.22.10 - ~/.npm-global/bin/yarn
    npm: 8.1.0 - /usr/local/bin/npm
  Browsers:
    Brave Browser: 88.1.20.110
    Chrome: 103.0.5060.114
    Firefox: 101.0.1
    Safari: 15.5

Used Package Manager

yarn

Logs

No response

Validations

• [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• [X] The provided reproduction is a minimal reproducible example of the bug.

[kompot]

I had create-react-app v4 ejected app and after upgrading to webpack 5 styles were not HMRed.

But following vanilla extract webpack integration docs fixed that.

Before, broken with webpack 5

{
  test: /\.vanilla\.css$/i,
  use: [
    isEnvDevelopment && require.resolve('style-loader'),
    isEnvProduction && {
      loader: MiniCssExtractPlugin.loader,
    },
    {
      loader: require.resolve('css-loader'),
      options: {
        // Required as image imports should be handled via JS/TS import statements
        url: false,
      },
    },
    postCssLoader,
  ].filter(Boolean),
},

After, working with webpack 5

{
  test: /\.vanilla\.css$/i,
  use: [
    MiniCssExtractPlugin.loader,
    {
      loader: require.resolve('css-loader'),
      options: {
        // Required as image imports should be handled via JS/TS import statements
        url: false,
      },
    },
    postCssLoader,
  ].filter(Boolean),
},

[danielberndt]

Thanks for the pointer @kompot. From my understanding, the config of the mono-repo should be sufficient. create-react-app already provides webpack with a rule for files ending in .css. The provided config works without issues in a non-mono-repo setup.

Out of curiosity I've still added a specific test: /\.vanilla\.css$/i, rule to webpack as you've proposed. And while that setup seems a bit more resilient, it only takes 4 or 5 css.js file changes until it breaks again unfortunately.

As far as I can tell from the reproduction, It seems to be specifically related to this part where a style from a different package is used within a style([]) call.

[danielberndt]

Okay, I've just finished a very deep dive into react-refresh and its interactions with the vanilla extract webpack plugin and found a very simple solution to the issue:

new VanillaExtractPlugin({externals: "@mono/ds/*"})

accordingly the solution for my original code base looks like this:

new VanillaExtractPlugin({externals: ["@name/pkg1/*", "@name/pkg1/**/*", "@name/pkg2/*", "@name/pkg2/**/*"]})

I had to locally patch the webpack plugin to accept an array for externals though. Happy to do a PR for this!

[danielberndt]

False positive unfortunately 🙁

Turns out it the stylesheet still gets removed after a few saves.

The most concrete thing I could find out in my research above is that react refresh looks for a ../dsStyes... module whereas there's only a ./dsStyles... in the module cache.

Sorry for all the noise!

[danielberndt]

Okay, I've spent pretty much the whole day trying to investigate further. I've created a branch with a minimal webpack repro and found the following: importing a css.js into both, another css.js and a non-css.js file leads to a race condition. It's a bit random which context triggers the first compilation. In a non-mono-repo it doesn't matter as the resulting requests are identical, but in the mono-repo setup the two contexts lead to slightly different webpack request.

../ds/dsStyles.css.js.vanilla.css!=!../node_modules/@vanilla-extract/webpack-plugin/virtualFileLoader/dist/[...]

vs

../ds/dsStyles.css.js.vanilla.css!=!../../node_modules/@vanilla-extract/webpack-plugin/virtualFileLoader/dist/[...]

even turning it into

../ds/dsStyles.css.js.vanilla.css!=!@vanilla-extract/webpack-plugin/virtualFileLoader[...]

doesn't solve the issue as I assume that webpack is doing some more directory resolution stuff later on.

If the order in which the two variants are loaded is changed, webpack's module cache receives an invalid moduleId causing it to crash.

So I experimented with guaranteeing that a file is only imported once via a simple cache here

https://github.com/seek-oss/vanilla-extract/blob/143bb45389620e97588b94ae06092ba8aa9b1e72/packages/webpack-plugin/src/loader.ts#L84

I couldn't find a good hook to trigger the cleaning of the cache, so I resorted to a hacky setTimeout, but the issue was gone and HMR worked like a charm for my code example.

I'm definitely lacking the context to know if this cache approach might break something else, but I do hope it helps in finding a more solid fix for this issue!

[danielberndt]

Okay, spent some more time, and Identified two issues:

1. if a css.js file is imported from different sub-folder depths (../file.css.js vs ../../file.css.js), it might result in a race condition that surfaces when hot-reloading
2. hot-updating a css.js file results in three webpack requests (css.js, css.js.vanilla.css, ./node-modules/css-loader/[...]vanilla.css). In bigger projects Webpack moves the latter into a separate vendors chunk. The hot reload mechanism assumes that all changes happen within the same chunk though, leading to a crash otherwise.

I tried to solve issue 1 with a cache (see comment above), but that's not necessary. Instead you can change this: https://github.com/seek-oss/vanilla-extract/blob/f33b6a5b2f15dbd3457e1d47eb06864c9498bf51/packages/webpack-plugin/src/loader.ts#L91-L94 into this:

const request = loaderUtils.stringifyRequest(
  this,
  `${path.resolve(fileName)}!=!${virtualResourceLoader}!${emptyCssExtractionFile}`,
);

i.e. replace fileName with path.resolve(fileName).

Issue number 2 can be solved by telling webpack to not put node-module requests into the vendors chunk if the request also includes vanilla-extract.

this is what my updated overrides-config.js looks like:

const {VanillaExtractPlugin} = require("@vanilla-extract/webpack-plugin");
const {getBabelLoader, removeModuleScopePlugin} = require("customize-cra");

module.exports = {
  webpack: function (config, env) {
    const loader = getBabelLoader(config);
    loader.options.plugins.push("@vanilla-extract/babel-plugin");
    config.plugins.push(new VanillaExtractPlugin());

    if (env === "development") {
      config.optimization = {
        ...config.optimization,
        splitChunks: {
          cacheGroups: {
            defaultVendors: {
              test: `[\\/]node_modules[\\/](?!.*vanilla-extract)`,
              priority: -10,
              reuseExistingChunk: true,
            },
            default: {
              minChunks: 2,
              priority: -20,
              reuseExistingChunk: true,
            },
          },
        },
      };
    }
    return removeModuleScopePlugin()(config);
  },
};

i.e. using test: '[\\/]node_modules[\\/](?!.*vanilla-extract)' to tell webpack to ignore vanilla-extract requests for the vendors chunk.

Applying both changes to our project results in a buttery smooth webpack 5 experience.

Happy to open a PR for issue 1. Issue 2 probably needs to be added to the docs. It might be the case that the config above can be simplified a lot. Not sure though whether this config wouldn't be beneficial for production-builds too (i.e. don't associate all css files with a single vendors-chunk)

[kasperstorgaard]

Fix 2 for HMR works very well for me too, thanks! 🥳