diff --git a/lib/loader.js b/lib/loader.js index d3f30ec2..333b545a 100644 --- a/lib/loader.js +++ b/lib/loader.js @@ -55,19 +55,10 @@ function sassLoader(content) { if (result.map && result.map !== "{}") { result.map = JSON.parse(result.map); - // result.map.file is an optional property that provides the output filename. - // Since we don't know the final filename in the webpack build chain yet, it makes no sense to have it. - delete result.map.file; - // The first source is 'stdin' according to node-sass because we've used the data input. - // Now let's override that value with the correct relative path. - // Since we specified options.sourceMap = path.join(process.cwd(), "/sass.map"); in normalizeOptions, - // we know that this path is relative to process.cwd(). This is how node-sass works. - result.map.sources[0] = path.relative(process.cwd(), resourcePath); - // node-sass returns POSIX paths, that's why we need to transform them back to native paths. - // This fixes an error on windows where the source-map module cannot resolve the source maps. - // @see https://github.com/webpack-contrib/sass-loader/issues/366#issuecomment-279460722 - result.map.sourceRoot = path.normalize(result.map.sourceRoot); - result.map.sources = result.map.sources.map(path.normalize); + // node-sass returns relative URL paths, not file system paths + // (POSIX or otherwise.) Aside from possible differences in the + // path separator, relative URL paths are (almost always) valid + // file system paths, allowing them to be used as such. } else { result.map = null; } diff --git a/lib/normalizeOptions.js b/lib/normalizeOptions.js index ad1ca1e2..e96d6fe4 100644 --- a/lib/normalizeOptions.js +++ b/lib/normalizeOptions.js @@ -32,26 +32,66 @@ function normalizeOptions(loaderContext, content, webpackImporter) { // Not using the `this.sourceMap` flag because css source maps are different // @see https://github.com/webpack/css-loader/pull/40 if (options.sourceMap) { - // Deliberately overriding the sourceMap option here. - // node-sass won't produce source maps if the data option is used and options.sourceMap is not a string. - // In case it is a string, options.sourceMap should be a path where the source map is written. - // But since we're using the data option, the source map will not actually be written, but - // all paths in sourceMap.sources will be relative to that path. - // Pretty complicated... :( - options.sourceMap = path.join(process.cwd(), "/sass.map"); - if ("sourceMapRoot" in options === false) { - options.sourceMapRoot = process.cwd(); - } - if ("omitSourceMapUrl" in options === false) { - // The source map url doesn't make sense because we don't know the output path - // The css-loader will handle that for us - options.omitSourceMapUrl = true; - } - if ("sourceMapContents" in options === false) { - // If sourceMapContents option is not set, set it to true otherwise maps will be empty/null - // when exported by webpack-extract-text-plugin. - options.sourceMapContents = true; - } + /** + * > Path to a file for LibSass to compile. + * @see node-sass [file]{@link https://github.com/sass/node-sass#file} + * + * > **Special behaviours** + * > + * > In the case that both file and data options are set, node-sass will + * > give precedence to data and use file to calculate paths in + * > sourcemaps. + * @see node-sass [Special behaviours]{@link https://github.com/sass/node-sass#special-behaviours} + * + * Another benefit to setting this is that `stdin`/`stdout` will no + * longer appear in `map.sources`. There will be no need to update + * `map.sources`, `map.names`, or similar. + * + * @type {String} [options.file=null] + */ + options.file = resourcePath; + + /** + * > **Special:** Required when `sourceMap` is a truthy value + * > + * > Specify the intended location of the output file. Strongly + * > recommended when outputting source maps so that they can properly + * > refer back to their intended files. + * > + * > **Attention** enabling this option will not write the file on disk + * > for you, it's for internal reference purpose only (to generate the + * > map for example). + * @see node-sass [outFile]{@link https://github.com/sass/node-sass#outfile} + * + * Even though we are always setting `sourceMap` to a string, the + * documentation says this is required, so set it to the expected value + * to comply with the requirement. + * + * `sass-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. + * + * @type {String | null} [options.outFile=null] + */ + options.outFile = resourcePath; + + /** + * > **Special: ** Setting the `sourceMap` option requires also setting + * > the `outFile` option + * > + * > Enables the outputting of a source map during `render` and + * > `renderSync`. When `sourceMap === true`, the value of `outFile` is + * > used as the target output location for the source map. When + * > `typeof sourceMap === "string"`, the value of `sourceMap` will be + * > used as the writing location for the file. + * @see node-sass [sourceMap]{@link https://github.com/sass/node-sass#sourcemap} + * + * @type {Boolean | String | undefined} [options.sourceMap=undefined] + */ + options.sourceMap = options.outFile + ".map"; } // indentedSyntax is a boolean flag. diff --git a/test/index.test.js b/test/index.test.js index cb5130a0..b55e0f86 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -204,18 +204,95 @@ implementations.forEach(implementation => { return fakeCwd; }; + /** + * Note: this test only tests the "standard" source map + * format. It does not test the "sections" source map + * format. + * + * @see Source Map Revision 3 Proposal's [Proposed Format]{@link https://docs.google.com/document/d/1U1RGAehQwRypUTovF1KRlpiOFze0b-_2gc6fAH0KY0k/edit#heading=h.qz3o9nc69um5} + */ return buildWithSourceMaps() .then(() => { + const resourcePath = path.join(__dirname, "scss", "imports.scss"); + const fileBase = path.basename(resourcePath); const sourceMap = testLoader.sourceMap; - sourceMap.should.not.have.property("file"); - sourceMap.should.have.property("sourceRoot", fakeCwd); - // This number needs to be updated if imports.scss or any dependency of that changes. - // Node Sass includes a duplicate entry, Dart Sass does not. - sourceMap.sources.should.have.length(implementation === nodeSass ? 11 : 10); - sourceMap.sources.forEach(sourcePath => - fs.existsSync(path.resolve(sourceMap.sourceRoot, sourcePath)) - ); + /** + * The value of the "sourceRoot" as passed to + * node-sass. + * + * @type {String|undefined|null} + */ + const sourceRootOption = undefined; + + /** + * The source map is a single JSON object. + */ + (sourceMap).should.be.an.Object(); + + /** + * Required. File version must be a positive + * integer. + */ + (sourceMap.version).should.be.a.Number().and.be.greaterThan(0); + (sourceMap.version % 1).should.equal(0); + + /** + * Optional. File is an optional name of the + * generated code that this source map is associated + * with. This is a URL string that is relative to + * source map. Typically it is just the file name + * and extension. + */ + if (sourceMap.file != null) { + (sourceMap.file).should.equal(fileBase); + } + + /** + * Optional. An optional source root, useful for + * relocating source files on a server or removing + * repeated values in the "sources" entry. This + * value is prepended to the individual entries in + * the "source" field. This is a URL string, + * undefined, or null. + */ + if (sourceRootOption != null) { + (sourceMap.sourceRoot).should.equal(sourceRootOption); + } else if (sourceMap.sourceRoot != null) { + (sourceMap.sourceRoot).should.be.a.String(); + } + + /** + * Required. Sources is an array original sources + * used by the "mappings" entry. Sources are + * URLs that are relative to the source map. + */ + (sourceMap.sources).should.be.an.Array(); + + /** + * Optional. An optional array of source content, + * useful when the "source" can’t be hosted. The + * contents are listed in the same order as the + * sources in the "sources" entry. "null" may be + * used if some original sources should be retrieved + * by name. + */ + if (sourceMap.sourcesContent != null) { + (sourceMap.sourcesContent).should.be.an.Array(); + (sourceMap.sourcesContent).should.have.length(sourceMap.sources.length); + } + + /** + * Required. Names is an array of symbol names used + * by the "mappings" entry. + */ + (sourceMap.names).should.be.an.Array(); + + /** + * Required. Mappings is a string with encoded + * mapping data. + */ + (sourceMap.mappings).should.be.a.String(); process.cwd = cwdGetter; });