Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aborting CSS HMR due to changed css-modules locals when using react-hot-loader #320

Closed
pattiereaves opened this issue Apr 24, 2018 · 26 comments · Fixed by #419
Closed

Aborting CSS HMR due to changed css-modules locals when using react-hot-loader #320

pattiereaves opened this issue Apr 24, 2018 · 26 comments · Fixed by #419

Comments

@pattiereaves
Copy link

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
When using the latest version of style-loader and react-hot-loader, the page will reload every time a change is made to a css file (regardless of the change being to a property or a selector). It provides the following error when doing a full page reload: Aborting CSS HMR due to changed css-modules locals. I believe this PR introduced the issue.

If the current behavior is a bug, please provide the steps to reproduce.
I am not entirely sure what about my configuration is causing this issue, but I suspect it is the combination of using css modules, react hot loader and style loader. See this comment. Downgrading to style-loader@0.19 fixes the issue. (Changes to CSS are applied to the page without a complete reload)

What is the expected behavior?
I can make changes to css and the changes will be applied using HMR.

If this is a feature request, what is motivation or use case for changing the behavior?
n/a

Please mention other relevant information such as your webpack version, Node.js version and Operating System.
node 8, npm 5
stylel-loader@0.21.0
"css-loader": "^0.28.11",
"react-hot-loader": "^4.0.1",
"webpack": "^4.2.0",
"webpack-dev-server": "^3.1.1"

@alexander-akait
Copy link
Member

@pattiereaves very strange, can you create minimum reproducible test repo?

@ghost
Copy link

ghost commented Jun 22, 2018

I've faced the same issue while using style-loader v0.21.0 alongside with webpack-hot-middleware.
Here is a small demo repo where this bug is reproduced.

@martinkadlec0
Copy link

Also just stumbled upon this problem, downgrading to 0.19 seems to fixed it.

@brigand
Copy link

brigand commented Jul 29, 2018

@lydell this seems related to #298 and there's a repro case linked here.

It also works for me when adding a local on 0.19.1, and your change was published in 0.20. I'm using babel-preset-react-hmre.

@asumaran
Copy link

asumaran commented Aug 22, 2018

I'm also getting the same error.

"style-loader": "^0.20.3",
"webpack-hot-middleware": "^2.22.2",
"react-hot-loader": "^4.2.0",

Error: Aborting CSS HMR due to changed css-modules locals.
    at main.js:157950
    at main.js:157953
    at Object.hotApply [as apply] (main.js:630)
    at cb (main.js:89783)
    at main.js:89799
onErrored @ main.js:89745
hotApply @ main.js:633
cb @ main.js:89783
(anonymous) @ main.js:89799
Promise.then (async)
check @ main.js:89798
../node_modules/webpack-hot-middleware/process-update.js.module.exports @ main.js:89759
processMessage @ main.js:89680
handleMessage @ main.js:89548
handleMessage @ main.js:89511

@pczern
Copy link

pczern commented Aug 25, 2018

Same error

@henrikbjorn
Copy link

We have also started to see this since upgrading to webpack 4 and related loaders etc.

@zeakd
Copy link

zeakd commented Sep 1, 2018

same error

  • "webpack-serve": "^2.0.2
  • "webpack": "^4.16.5"
  • "style-loader": "^0.22.1"

This occurs when create selector, and add properties to empty selector.

// create
.text-input {
  margin-top: 0.4em;
} 
// Error: Aborting CSS HMR due to changed css-modules locals.
.text-input {

}
// add new properties
.text-input {
  margin-top: 0.4em;
} 
// Error: Aborting CSS HMR due to changed css-modules locals.

HMR still works when adding new properties to selector which have one or more properties.

.text-input {
  margin-top: 0.4em;
}
// add new properties
.text-input {
  margin-top: 0.4em;
  margin-bottom: 0.4em;
} 
// hmr works.

@coreyward
Copy link

I'm also seeing the behavior that @zeakd detailed where HMR with CSS modules works fine part of the time (specifically, as long as the changes do not result in a selector being added or a selector without any declarations having one or more added). This is despite not actually using any variables at all, which makes the message rather confusing.

Running style-loader v0.21.0.

@githubflub
Copy link

Fix this please.

@lydell
Copy link
Contributor

lydell commented Sep 18, 2018

Just to give some context:

Before #298, without react-hot-loader

Changing anything in a CSS file caused a full page reload! Super sad!

After #298, without react-hot-loader

Changing properties in CSS hot reloads! Yay!

But if classes are added, removed or changed the whole page is reloaded. Why? Because your React render methods need to run again with the new classes.

Before #298, with react-hot-loader

I guess CSS changes were always hot reloaded?

After #298, with react-hot-loader

Hot reloading fails (requiring manual page reload?) if classes are added, removed or changed? Have I understood that correctly?

Why does this happen? Because of #298 (comment):

// This error is caught and not shown and causes a full reload.
throw new Error('Aborting CSS HMR due to changed css-modules locals.');

lydell on Jan 26 (Contributor)

I looked through https://webpack.js.org/api/hot-module-replacement/ and tried to read the HMR source code in the webpack repo, and could not find any other way of aborting an accept other than throwing an error. It seems to work flawlessly, though!

michael-ciniawsky on Jan 26 (Member)

I will take a look into this, I think it's fine

My guess

react-hot-loader needs to catch the thrown error and ignore it, because since the React code is hot loaded the render methods always re again with the new classes so the case doesn't apply.

So it might be that something needs to be done in the react-hot-loader repo, not this one! Or, if that doesn't work out, maybe we need an option?

@lydell
Copy link
Contributor

lydell commented Sep 18, 2018

I took a closer look at @tpolovyi's demo (from #320 (comment)) (https://github.com/tpolovyi/style-loader-demo) and it does not involve react-hot-loader at all!

The difference compared to my demo used to test the PR (https://github.com/lydell/css-hmr-bug/tree/fix) is that I use webpack-dev-server while @tpolovyi uses webpack-dev-middleware directly in express. And in his example, the throw approach doesn't work! (The error is just logged in the console instead of causing a page reload.)

I think we need help from somebody who deeply understands how HMR works in webpack.

@beshanoe
Copy link

@lydell, can you please explain again why do we need to do full reload when we added a new rule?
here's my use case:

/* Button.css */
.Foo {
  color: red;
}
import React from 'react'
import styles from './Button.css'

export default class Button extends React.Component {
  render() {
    return <div className={styles.Foo}>{this.props.children}</div>
  }
}

Before your PR, I could add a new class .Bar to the Button.css and then added it to my div's className. None of that was causing full-page reload. And now it broke completely since we use webpack-hot-middleware(as a lot of people).
My opinion that it's worth a hotfix for backward compatibility(like an option flag for a loader) or a major release

@alexander-akait
Copy link
Member

Somebody can create minimum reproducible test repo and describe in readme what actually happens and what expected (and what was in old version)?

@beshanoe
Copy link

@alexander-akait
Copy link
Member

alexander-akait commented Dec 24, 2018

@beshanoe thanks, after holidays we will refactor style-loader (and solve this issue too)

@lydell
Copy link
Contributor

lydell commented Dec 26, 2018

@beshanoe If you first add className={styles.Foo} in your JS and then define .Foo in your CSS the page needs to be reloaded, right? Unless you're using react-hot-loader? But maybe I didn't think it through enough? The previous behavior was to always reload the page if CSS modules was enabled. I changed it to sometimes reload. But maybe it makes sense to never reload?

@beshanoe
Copy link

@lydell yeah I understand there are bunch of edge cases and I can't say for sure which solution gonna cover them all, but when using react-hot-loader with CSS modules and webpack-hot-middleware, it worked before and now it doesn't, at least because apparently webpack-hot-middleware doesn't support reloading on throwed error.
I think your change makes total sense if it fixes HMR for those who don't use RHL, but for those who uses, RHL does all the work, so to me the option flag will work the best, since there are already a lot of them in the style-loader

@beshanoe
Copy link

beshanoe commented Dec 27, 2018

@lydell actually I was completely ignoring the fact that HMR was turned off for css modules before, so for those who uses them only in react app with RHL, current fix is to provide hmr: false to style-loader:

{
  test: /\.css$/,
  use: [
    {
      loader: "style-loader",
      options: {
        hmr: false
      }
    },
    {
      loader: "css-loader",
      options: {
        modules: true
      }
    }
  ]
}

and make react-hot-loader do the job

@ccpu
Copy link

ccpu commented Dec 30, 2018

tried @beshanoe solution and disabled hmr in "style-loader"; it works, however, it causes react rerender every time scss content changed.

downgrade package to 0.19.1 worked for me.

@alexander-akait
Copy link
Member

@cyrus-d can you create minimum reproducible test repo?

@ccpu
Copy link

ccpu commented Jan 4, 2019

@evilebottnawi this is the boilerplate that I have modified to reproduce the problem, please look into the webpack/modules.js for configurations:

https://github.com/Cyrus-d/webpack-react-boilerplate

This is what happens:

Test

hope it helps.

@beshanoe
Copy link

beshanoe commented Jan 4, 2019

Hey @cyrus-d
I see you used hmr option in the wrong place, you've put it inside css-loader's option here:
https://github.com/Cyrus-d/webpack-react-boilerplate/blob/fefcf42626e2573e36ba2239fca3a44244e235d7/webpack/modules.js#L34
instead of style-loader's options here:
https://github.com/Cyrus-d/webpack-react-boilerplate/blob/fefcf42626e2573e36ba2239fca3a44244e235d7/webpack/modules.js#L41

@ccpu
Copy link

ccpu commented Jan 5, 2019

@beshanoe thanks for pointing that out, actually at first, I did had hmr option under style-loader, but it caused react rerender the app, look into it closely I have noticed that the react hot loader was configured incorrectly. so yes, setting hmr to false indeed solved the problem.

Thanks for the help.

pfdgithub added a commit to pfdgithub/scaffold-webpack that referenced this issue Jan 7, 2019
@mikestopcontinues
Copy link

Hey, where are we at with this issue? I've downgraded for the time being, but I sure would like to get back on the update track and still have hmr.

@alexander-akait
Copy link
Member

@martinkadlec0 PR welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.