Add Option To Fix Relative Urls

[bendytree]

When sourceMaps are enabled, relative urls stop working. This appears to be a pain point for a lot of people. (Ref 1, 2, 3, 4, 5, 6, 7, 8, 9, etc)

Basically style-loader uses a link data-uri to embed the css with a source map. That means relative urls have no way to resolve their path.

Ideally urls should be converted to absolute urls before the style-loader runs, but that isn't always possible. In my opinion style-loader is in a unique position to alleviate this issue.

This pull request creates an opt-in option fixUrls. When enabled, it uses regex to replace relative image/font urls with absolute urls according to the page location.

Ideally I'd parse the css, but client-side parsers look to be 40-80KB and would likely be a lot slower. Parsing has to happen client side because the location is not known until then.

I've included about 30 test (which can be run with npm test) to verify that relative urls are resolved properly, absolute urls are not touched, etc.

[Strate]

Your patch has a definetely better quality than my, great work!
But, from my point of view, there is no reason to have an option fixUrls. I can not imagine situation, when any developer want to turn it off.
Alternatively, I can suggest to keep it option enabled by default, to get working loader out of the box, with option to disable that feature in some rare cases.

[bendytree]

I tend to agree. What's the point of having an option that basically means "don't be broken"?

On the other hand, I can see how people would be squeamish about mucking up their css with regex, so I figured that opt-in would give the PR a better shot.

I'm happy to remove the option or make it opt-out.

[Strate]

people would be squeamish about mucking up their css with regex

postcss actually parses css via regexp, so, if anyone don't like regexp, he shouldn't use css-loader at all.

[SimonSelg]

Any update on this PR getting merged or not? :)

[valerybugakov]

+1 any update?

[Jenselme]

👍 This works like a charm.

[volodymyr-tsaryk]

+1 for this fix

[gimdongwoo]

It's good solution for #96

Thank you!!

[rhagigi]

Any maintainers here? Please let this land

[tlaak]

Thank you so much for this!

[seansfkelley]

I just spent 3 hours debugging this exact issue and would absolutely have loved this option.

[samhh]

This really deserves merging. Fixes the issue well.

[kohlmannj]

This PR has no labels, no comments from the Webpack team, and subsequently no visibility into when a fix of this nature might make it into style-loader. Would really appreciate comment, @sokra et al, please and thank you! I and another coworker spent a few hours trying to debug this problem.

[rhagigi]

This PR has gotten +1s from so many people, and no code-commit has been made to this repo in almost 9 months. It's sad that a package used by almost every user of webpack - the base loader for almost every css/sass/less/style-related loader - has gone unmaintained like this for this long, with multiple viable and useful fixes sitting in PRs, especially this one.

[sokra]

I'll put it on my TODO list. style-loader will be one of the things I'll care about after my vacation...

[kohlmannj]

@sokra My sincere thanks for your reply! Please enjoy your vacation 🏖

[romulof]

Made some modifications to support URLs with hashes and id references (common on SVGs).

https://github.com/romulof/style-loader/

[samhh]

Edit: Ignore my previous derp. @bendytree and @romulof's forks are both great though for the issue in question 👍

[romulof]

@samhh, can you post your CSS/Sass/whatever input, CSS output, base URL and webpack publicUrl?

[samhh]

@romulof I knew it was going to be something obvious I was cocking up... and it was... there was an issue with the font itself. My apologies! Keep up the great work :-)

[JimmyMultani]

@sokra hope you had a great vacation. Any news on whether this will be merged or not?

[protoEvangelion]

@sokra Are there any current workarounds that you would recommend?

[sontek]

What the status of this?

[romulof]

@sokra might be busy with webpack 2 release.

[sontek]

If it helps, I'm using ?fixUrls from "style-loader": "github:bendytree/style-loader", and it works beautifully.

[john-bai]

As much as I like this PR I think there might be a better solution. I did a little digging around and saw that @sokra added styles differently when source maps were enabled because, at the time, I believe source maps didn't work when referred inside a style tag (his comment here: #30 (comment))

This bug was fixed in Chromium in early 2015 and has likely made it to Chrome by now: https://bugs.chromium.org/p/chromium/issues/detail?id=456062

So I think the best solution at this point would be removing the link element + Blob-based object url approach from style-loader.

@sokra thoughts? I'd be willing to work on a PR.

[john-bai]

Ok, I've spent hours and came up with a conclusion. This PR is the best solution at this time for style-loader (if @bendytree merges in @romulof's bug fix to support urls with hash fragments) . However, once Chrome Canary 57.0.2981.0's functionality makes it into latest Chrome, we won't need this PR and could simplify how style-loader adds styles and source map and it will all just work, relative urls and all (I made these changes in #165). style-loader would only need to add the styles and sourceMappingURL directive directly in a style element as text. Chrome Canary is paying attention to changes in style elements and will load a new source map if you provide a sourceMappingURL directive. I'm going to submit a separate PR that'll work in the near future.

Here's a summary of my findings as I went about trying to implement an alternative solution:

1. In latest chrome, you can put a sourceMappingURL inside a style tag and it will work as long as the style element wasn’t added to the DOM via javascript.
   1. NOTE: the sourceMappingURL directive needs to be a sibling of the actual CSS style declarations.
2. You can use an application/json;base64 data URI to store the source map file:
   1. /*# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJtYXBwaW5ncyI6IkFBS0EsRUFBRztFQUNDLFNBQVMsRUFORixJQUFJO0VBT1gsS0FBSyIsInNvdXJjZXMiOlsic2Fzcy9zdHlsZXMuc2NzcyJdLCJmaWxlIjoic3R5bGVzLmNzcyJ9 */
3. If you add the sourceMappingURL directive in a style element via javascript, the source map doesn’t load in latest chrome or safari.
   1. Works in Firefox 50.1.0.
   2. Works in Chrome Canary 57.0.2981.0
4. If you add an import to a base64 encoded data-uri containing the styles and sourceMappingURL, the source map will load in latest chrome. However, relative urls and protocol-relative urls in styles don’t work. This was the original approach style-loader used to get Chrome to load a source map dynamically.

[john-bai]

@sokra Can you please chime in now and make a decision to accept or reject this PR? We've waited 8 months and several of us are moving to forks. How would you like to address this issue? Do you believe this feature should be part of style-loader? If not, what would you recommend instead? Please also consider #165 as an alternative (might be more appropriate for the future).

[donaldpipowitch]

@john-bai Thank you for your progress on this issue.

@SpaceK33z @sokra or @TheLarkInn This seems to be the most discussed PR on style-loader. It looks like @john-bai put really a lot of effort to investigate in this issue. Maybe you can give him or someone else push rights or something like that to get some more help reviewing/maintaining? :)

[sontek]

What is the status of this? We've stopped upgrading style-loader because we have to use the bendytree fixUrls branch. @bendytree Maybe you could merge your branch with mater so we can have the latest + your fix?

[joshwiens]

1.) Rebase in current master please.
2.) If you need something addressed use @webpack-contrib/org-maintainers so you are pinging the right people.
3.) Between this #165 we have a fix & a future proof feature. I'm personally leaning towards landing this sooner than later as a patch to solve current pains and including #165 in the next Major as it is still pending on browser support.

//cc @bebraw @ekulabuhov

[sontek]

It looks like someone else is going to have to take this on. @bendytree hasn't been around since the original PR a year ago

[joshwiens]

@sontek - Thanks for the heads up, i'll get it taken care of.

[sontek]

I pulled down his changes and rebased them on master as #186

[joshwiens]

Closing this in favor of #186

[cbonita13]

#186