fix(loader): source map sources and file paths

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;
         }

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.

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;
                         });