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

Avoid HMR crash when src is undefined #508

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ro-savage
Copy link

This PR contains a:

  • 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

@jsf-clabot
Copy link

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.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add test or create reproducible test repo and I will write test

@ro-savage
Copy link
Author

ro-savage commented Mar 13, 2020

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.

@AntonioRedondo
Copy link

AntonioRedondo commented Apr 18, 2020

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

@ro-savage
Copy link
Author

@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",

@lgollut
Copy link

lgollut commented Jun 16, 2020

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

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

Successfully merging this pull request may close these issues.

None yet

5 participants