-
-
Notifications
You must be signed in to change notification settings - Fork 604
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
fix: source map sources
and file
paths
#753
Changes from 2 commits
e4497c6
7f4f7a8
8cf05a9
027c8ca
32e7f46
c0669ab
ffdd935
16c317a
68bd963
26680b0
c2e272f
625ffb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,18 +2,39 @@ | |
MIT License http://www.opensource.org/licenses/mit-license.php | ||
Author Tobias Koppers @sokra | ||
*/ | ||
var path = require("path"); | ||
var loaderUtils = require("loader-utils"); | ||
var processCss = require("./processCss"); | ||
var getImportPrefix = require("./getImportPrefix"); | ||
var compileExports = require("./compile-exports"); | ||
|
||
/** | ||
* If the file was "renamed" (for the purposes of source maps), then honor it, | ||
* otherwise just use the resourcePath. | ||
* @param {String} resourcePath - The absolute file system path for the sass file. | ||
* @param {Object|null} map - An existing source map, if any. | ||
* @return {String} - The effective path to use for the `from` argument. | ||
*/ | ||
function processFrom(resourcePath, map) { | ||
var effectiveResourcePath; | ||
if (map && map.file && typeof map.file === 'string' && path.dirname(map.file) === '.') { | ||
// Something else has already changed the file name or extension, so | ||
// honor it for the purpose of creating the next source map. | ||
effectiveResourcePath = path.join(path.dirname(resourcePath), map.file); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please provide an example for |
||
} else { | ||
effectiveResourcePath = resourcePath; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick 🐦 |
||
return effectiveResourcePath; | ||
} | ||
|
||
module.exports = function(content, map) { | ||
var callback = this.async(); | ||
var query = loaderUtils.getOptions(this) || {}; | ||
var moduleMode = query.modules; | ||
var camelCaseKeys = query.camelCase; | ||
var sourceMap = query.sourceMap || false; | ||
var processCssFrom; | ||
var processCssTo; | ||
|
||
if(sourceMap) { | ||
if (map) { | ||
|
@@ -33,10 +54,37 @@ module.exports = function(content, map) { | |
map = null; | ||
} | ||
|
||
if (query.legacySourceMaps !== false) { | ||
processCssFrom = loaderUtils.getRemainingRequest(this).split("!").pop(); | ||
processCssTo = loaderUtils.getCurrentRequest(this).split("!").pop(); | ||
} else { | ||
|
||
/** | ||
* > To ensure that PostCSS generates source maps and displays better syntax | ||
* > errors, runners must specify the from and to options. If your runner | ||
* > does not handle writing to disk (for example, a gulp transform), you | ||
* > should set both options to point to the same file | ||
* @see postcss [PostCSS Runner Guidelines]{@link https://github.com/postcss/postcss/blob/master/docs/guidelines/runner.md#21-set-from-and-to-processing-options} | ||
* | ||
* `css-loader` isn't responsible for writing the map, so it doesn't have to | ||
* worry about updating the map with a transformation that changes locations | ||
* (suchs as map.file or map.sources). | ||
* | ||
* Changing the file extension counts as changing the location because it | ||
* changes the path. | ||
* | ||
* PostCSS's `from` and `to` arguments are only concerned with the file | ||
* system. They don't know about, care about, or understand the webpack | ||
* loader's current request or remaining request. | ||
*/ | ||
processCssFrom = processFrom(this.resourcePath, map); | ||
processCssTo = processCssFrom; | ||
} | ||
|
||
processCss(content, map, { | ||
mode: moduleMode ? "local" : "global", | ||
from: loaderUtils.getRemainingRequest(this).split("!").pop(), | ||
to: loaderUtils.getCurrentRequest(this).split("!").pop(), | ||
from: processCssFrom, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. {
...,
// TODO use shorthand in next major
from: from,
to: to
} |
||
to: processCssTo, | ||
query: query, | ||
loaderContext: this, | ||
sourceMap: sourceMap | ||
|
@@ -112,13 +160,15 @@ module.exports = function(content, map) { | |
if(sourceMap && result.map) { | ||
// add a SourceMap | ||
map = result.map; | ||
if(map.sources) { | ||
npetruzzelli marked this conversation as resolved.
Show resolved
Hide resolved
|
||
map.sources = map.sources.map(function(source) { | ||
return source.split("!").pop().replace(/\\/g, '/'); | ||
}, this); | ||
map.sourceRoot = ""; | ||
if (query.legacySourceMaps !== false) { | ||
if(map.sources) { | ||
map.sources = map.sources.map(function(source) { | ||
return source.split("!").pop().replace(/\\/g, '/'); | ||
}, this); | ||
map.sourceRoot = ""; | ||
} | ||
map.file = map.file.split("!").pop().replace(/\\/g, '/'); | ||
} | ||
map.file = map.file.split("!").pop().replace(/\\/g, '/'); | ||
map = JSON.stringify(map); | ||
moduleJs = "exports.push([module.id, " + cssAsString + ", \"\", " + map + "]);"; | ||
} else { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path.dirname(map.file) === '.'
checks for a relative path here ? I'm not sure I'm following.. 😛 . In case my understanding is correct here, there is alsopath.isAbsolute(filepath)
availableThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done some thinking on it and here I may be trying to be too clever. What I originally was thinking of shouldn't be something that css-loader is responsible for. I can provide more details on what I was thinking if you'd like.
I'll try cleaning this up in my next commit.
will be enough.