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

[hmr] reloadStyle will trigger errors #377

Closed
hsxfjames opened this issue Apr 13, 2019 · 13 comments · Fixed by #402
Closed

[hmr] reloadStyle will trigger errors #377

hsxfjames opened this issue Apr 13, 2019 · 13 comments · Fixed by #402

Comments

@hsxfjames
Copy link

hsxfjames commented Apr 13, 2019

  • Operating System: Mac OS 10.14.4
  • Node Version: v11.6.0
  • NPM Version: 6.9.0
  • webpack Version: 4.30.0
  • mini-css-extract-plugin Version: 0.6.0

Expected Behavior

If we have a link in document which its href attribute is in data url form, reloadStyle() function shouldn't select and parse it. For example:

<link rel="shortcut icon" href="data:;base64,=">

Actual Behavior

reloadStyle() function will select this link element and try to parse its href, then throws an error from getReloadUrl() function.

Uncaught TypeError: Failed to construct 'URL': Invalid URL

Code

no need?

How Do We Reproduce?

Add the link mentioned above to any html template.

@alexander-akait
Copy link
Member

/cc @ScriptedAlchemy I think it is easy to fix

@ScriptedAlchemy
Copy link
Contributor

Looking into it now, hope to have PR ready soon. This weekend is a little busy

@ScriptedAlchemy
Copy link
Contributor

@hsxfjames do you want to give this a try. #378

@alexander-akait
Copy link
Member

Fixed #378

@hsxfjames
Copy link
Author

Not works, because getReloadUrl() at line#148 goes before if (!isUrlRequest(url)).

Consider to judge href in raw? Or how about matching links those with rel=stylesheet only?

@hsxfjames
Copy link
Author

Oh, am I late?😂

@alexander-akait
Copy link
Member

rel can be omitted, what is problem?

@alexander-akait
Copy link
Member

@hsxfjames fix doesn't work for you?

@hsxfjames
Copy link
Author

@evilebottnawi
Not works, it still throws errors when I try to modify css. The error is from reloadStyle -> getReloadUrl -> normalizeUrl -> new URL().
I am in a limited network, so I'm also hard to paste full error message here. But you can easily try to reproduce this, just to add a simple link in base64 url form into any template you like. Let me know if you need other information~

@alexander-akait
Copy link
Member

@hsxfjames thanks
/cc @ScriptedAlchemy

@ScriptedAlchemy
Copy link
Contributor

@hsxfjames will work on this tonight, then ask you to test it again

@ScriptedAlchemy
Copy link
Contributor

looks like part of this fix broke some HMR for many @evilebottnawi

faceyspacey/extract-css-chunks-webpack-plugin#170

I removed the line, so help my users - i am very bad with regex. Any ideas on what we need to do

@alexander-akait
Copy link
Member

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.

3 participants