refactor: modifying source map support to also support relative urls in stylesheets #165
Conversation
Change Readme to use new Rule.use configuration
… when source maps are enabled. Chrome Canary 57.0.2981.0 and Firefox 50.1.0 support loading source maps when sourceMapURL directive is dynically added to a new or existing style element in the DOM. This means that style-loader can simply add the styles and sourceMapURL directive as text inside a style element and the source map will be loaded by the browser. Given the simpler insertion method, the relative and protocol-relative urls in stylesheets will not break.
|
|
john-bai
changed the title
…ateDocsForFinal chore(readme): Update to webpack standard format
To allow inclusion in https://webpack.js.org/ docs.
Remove license link
Update the link to css-loader local scope
Fixed link to local scope
…rning fix deprecated warning
There was a problem hiding this comment.
Plenty of discussion on this bug in #124. This PR would be great to merge in once browser support is at an acceptable level. Currently Chrome Canary 57.0.2981.0 and Firefox 50.1.0 work with this source map support.
I think this has to wait till the browsers have caught up.
Great work, though, as it cuts a lot of code. Tests would be good to add.
|
@john-bai This needs to be rebased to pick up changes in master |
|
This also supersedes/fixes #124. Chrome v57 should enter stable somewhere at the end of this month. Firefox is at v51 already. |
|
@john-bai - Chrome 57 global rollout started a few hours ago, can you get this rebased so we can get this PR on deck.
|
|
@john-bai Can you rebase? Thanks. |
michael-ciniawsky
changed the title
|
This PR would also resolve #121; I verified that this issue is caused by using blobs and it doesn't exists when bare Is anything except a rebase needed to land & release this? Some tests? |
The author needs to sign the CLA before we can merge && move on with this PR (even if unfinished) or someone picks the changes here and opens a new PR :) Test(s) would be appreciated This change is definitely needed |
|
@michael-ciniawsky As I understand, the author did sign the CLA. The checker failed because of a merge commit in this PR that pulled in changes by other people not all of whom signed the new CLA. |
|
@mgol That could get annoying maybe the PR even needs to be reopened enterily. As I understand the CLA Bot checks the commit authors and there is nearly no way to fix it :). If you have time and interest feel free to open a new PR with these changes |
|
@michael-ciniawsky I rebased it, see #219. I can add proper tests, just please comment on what else might be needed to land it. |
|
Superseded by #219 |
Fixes #165 webpack-contrib/style-loader#165 Using #219 refactor: use annotation comments for source maps (`options.sourceMap`) webpack-contrib/style-loader#219
Fixing broken relative and protocol-relative url() use in stylesheets when source maps are enabled. Chrome Canary 57.0.2981.0 and Firefox 50.1.0 support loading source maps when sourceMapURL directive is dynamically added to a new or existing style element in the DOM. This means that style-loader can simply add the styles and sourceMapURL directive as text inside a style element and the source map will be loaded by the browser. Given the simpler insertion method, the relative and protocol-relative urls in stylesheets will not break.
Plenty of discussion on this bug in #124. This PR would be great to merge in once browser support is at an acceptable level. Currently Chrome Canary 57.0.2981.0 and Firefox 50.1.0 work with this source map support.
README updated to remove the warning about source maps and relative urls in styles breaking.