mini-css-extract-plugin icon indicating copy to clipboard operation
mini-css-extract-plugin copied to clipboard

Avoid HMR crash when src is undefined

Open ro-savage opened this issue 5 years ago • 5 comments

This PR contains a:

  • [x] bugfix
  • [ ] new feature
  • [ ] code refactor
  • [ ] test update
  • [ ] typo fix
  • [ ] metadata update

Motivation / Use-Case

The function returned by getCurrentScriptUrl usually returns an array. However when !src is true it returns null.

This causes a crash when getReloadUrl() is run and tries to execute src.some((url) =>)

https://github.com/webpack-contrib/mini-css-extract-plugin/blob/master/src/hmr/hotModuleReplacement.js#L137

This instead returns an empty array instead so that it can always safely be assumed that an array will be returned.

I couldn't find any reason to return null instead, but if there is a reason. Another option would be to have a guard before or inside reloadStyle.

Breaking Changes

None that I am aware of.

Additional Info

Log from our setup. This happens every time we change a CSS file, and occasionally when just changing a JS file.

hotModuleReplacement.js:130 Uncaught TypeError: Cannot read property 'some' of null
    at getReloadUrl (hotModuleReplacement.js:130)
    at hotModuleReplacement.js:148
    at NodeList.forEach (<anonymous>)
    at reloadStyle (hotModuleReplacement.js:142)
    at update (hotModuleReplacement.js:197)
    at functionCall (hotModuleReplacement.js:24)
getReloadUrl @ hotModuleReplacement.js:130
(anonymous) @ hotModuleReplacement.js:148
reloadStyle @ hotModuleReplacement.js:142
update @ hotModuleReplacement.js:197
functionCall @ hotModuleReplacement.js:24
setTimeout (async)
(anonymous) @ hotModuleReplacement.js:28
hotApply @ bootstrap:607
(anonymous) @ bootstrap:362
Promise.then (async)
hotUpdateDownloaded @ bootstrap:361
hotAddUpdateChunk @ bootstrap:337
webpackHotUpdateCallback @ bootstrap:57
(anonymous) @ application-874885fd2d95998bf4fc.hot-update.js:1

ro-savage avatar Mar 12 '20 22:03 ro-savage

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

jsf-clabot avatar Mar 12 '20 22:03 jsf-clabot

Unfortunately, I haven't been able to track down why we hit https://github.com/webpack-contrib/mini-css-extract-plugin/blob/master/src/hmr/hotModuleReplacement.js#L54 and have null returned. So I haven't been able to create a demo repo (we are using rails and webpacker to run webpack).

However, if you reach L54 for any reason then it will crash the webpack because https://github.com/webpack-contrib/mini-css-extract-plugin/blob/master/src/hmr/hotModuleReplacement.js#L137 will always throw an error trying to runn null.some().

It appears null is the incorrect thing to return when no src is found, instead an empty array makes more sense.

ro-savage avatar Mar 13 '20 10:03 ro-savage

Any progress on this PR? I'm experiencing the error and sadly it prevents me to hot reload styling. 😢

AntonioRedondo avatar Apr 18 '20 19:04 AntonioRedondo

@AntonioRedondo - I run a forked version now, since this hasn't been fixed. Given the fact this does fix it, and null is obviously incorrect since later on it runs .some() on the value. I don't know why this hasn't been fixed/merged. Regardless of the test case or not, you can literally just read the code and see this path will always throw. @evilebottnawi

You can run the forked version as "mini-css-extract-plugin": "npm:@ro-savage/mini-css-extract-plugin",

ro-savage avatar Apr 19 '20 02:04 ro-savage

We experience that as well with some vue component that dosen't contains any style section... Will try to track down a little if I can

lgollut avatar Jun 16 '20 09:06 lgollut