feat: source map generation depends on the devtool option (#743)

BREAKING CHANGE: default value of the `sourceMap` option depends on the `devtool` value (`eval`/`false` values don't enable source map generation)
webpack-contrib · Aug 23, 2019 · fcea88e · fcea88e
1 parent 33a23f8
commit fcea88e
Show file tree

Hide file tree

Showing 5 changed files with 157 additions and 34 deletions.
diff --git a/README.md b/README.md
@@ -363,11 +363,11 @@ module.exports = {
 ### `sourceMap`
 
 Type: `Boolean`
-Default: `false`
+Default: depends on the `compiler.devtool` value
 
 Enables/Disables generation of source maps.
 
-They are not enabled by default because they expose a runtime overhead and increase in bundle size (JS source maps do not).
+By default generation of source maps depends on the [`devtool`](https://webpack.js.org/configuration/devtool/) option, all values enable source map generation except `eval` and `false` value.
 
 **webpack.config.js**
 

diff --git a/src/getSassOptions.js b/src/getSassOptions.js
@@ -41,10 +41,15 @@ function getSassOptions(loaderContext, loaderOptions, content) {
     options.outputStyle = 'compressed';
   }
 
+  const useSourceMap =
+    typeof loaderOptions.sourceMap === 'boolean'
+      ? loaderOptions.sourceMap
+      : loaderContext.sourceMap;
+
   // opt.sourceMap
   // Not using the `this.sourceMap` flag because css source maps are different
   // @see https://github.com/webpack/css-loader/pull/40
-  if (loaderOptions.sourceMap) {
+  if (useSourceMap) {
     // 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.

diff --git a/src/index.js b/src/index.js
@@ -62,7 +62,7 @@ function loader(content) {
   render(sassOptions, (error, result) => {
     if (error) {
       if (error.file) {
-        this.dependency(error.file);
+        addNormalizedDependency(error.file);
       }
 
       callback(new SassError(error, this.resourcePath));

diff --git a/test/__snapshots__/sourceMap-options.test.js.snap b/test/__snapshots__/sourceMap-options.test.js.snap
@@ -1,49 +1,97 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`sourceMap option false (dart-sass) (sass): errors 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "false" value (dart-sass) (sass): errors 1`] = `Array []`;
 
-exports[`sourceMap option false (dart-sass) (sass): warnings 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "false" value (dart-sass) (sass): warnings 1`] = `Array []`;
 
-exports[`sourceMap option false (dart-sass) (scss): errors 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "false" value (dart-sass) (scss): errors 1`] = `Array []`;
 
-exports[`sourceMap option false (dart-sass) (scss): warnings 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "false" value (dart-sass) (scss): warnings 1`] = `Array []`;
 
-exports[`sourceMap option false (node-sass) (sass): errors 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "false" value (node-sass) (sass): errors 1`] = `Array []`;
 
-exports[`sourceMap option false (node-sass) (sass): warnings 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "false" value (node-sass) (sass): warnings 1`] = `Array []`;
 
-exports[`sourceMap option false (node-sass) (scss): errors 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "false" value (node-sass) (scss): errors 1`] = `Array []`;
 
-exports[`sourceMap option false (node-sass) (scss): warnings 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "false" value (node-sass) (scss): warnings 1`] = `Array []`;
 
-exports[`sourceMap option not specify (dart-sass) (sass): errors 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "source-map" value (dart-sass) (sass): errors 1`] = `Array []`;
 
-exports[`sourceMap option not specify (dart-sass) (sass): warnings 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "source-map" value (dart-sass) (sass): warnings 1`] = `Array []`;
 
-exports[`sourceMap option not specify (dart-sass) (scss): errors 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "source-map" value (dart-sass) (scss): errors 1`] = `Array []`;
 
-exports[`sourceMap option not specify (dart-sass) (scss): warnings 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "source-map" value (dart-sass) (scss): warnings 1`] = `Array []`;
 
-exports[`sourceMap option not specify (node-sass) (sass): errors 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "source-map" value (node-sass) (sass): errors 1`] = `Array []`;
 
-exports[`sourceMap option not specify (node-sass) (sass): warnings 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "source-map" value (node-sass) (sass): warnings 1`] = `Array []`;
 
-exports[`sourceMap option not specify (node-sass) (scss): errors 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "source-map" value (node-sass) (scss): errors 1`] = `Array []`;
 
-exports[`sourceMap option not specify (node-sass) (scss): warnings 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value has "true" value and the "devtool" option has "source-map" value (node-sass) (scss): warnings 1`] = `Array []`;
 
-exports[`sourceMap option true (dart-sass) (sass): errors 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value is not specify and the "devtool" option has "source-map" value (dart-sass) (sass): errors 1`] = `Array []`;
 
-exports[`sourceMap option true (dart-sass) (sass): warnings 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value is not specify and the "devtool" option has "source-map" value (dart-sass) (sass): warnings 1`] = `Array []`;
 
-exports[`sourceMap option true (dart-sass) (scss): errors 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value is not specify and the "devtool" option has "source-map" value (dart-sass) (scss): errors 1`] = `Array []`;
 
-exports[`sourceMap option true (dart-sass) (scss): warnings 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value is not specify and the "devtool" option has "source-map" value (dart-sass) (scss): warnings 1`] = `Array []`;
 
-exports[`sourceMap option true (node-sass) (sass): errors 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value is not specify and the "devtool" option has "source-map" value (node-sass) (sass): errors 1`] = `Array []`;
 
-exports[`sourceMap option true (node-sass) (sass): warnings 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value is not specify and the "devtool" option has "source-map" value (node-sass) (sass): warnings 1`] = `Array []`;
 
-exports[`sourceMap option true (node-sass) (scss): errors 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value is not specify and the "devtool" option has "source-map" value (node-sass) (scss): errors 1`] = `Array []`;
 
-exports[`sourceMap option true (node-sass) (scss): warnings 1`] = `Array []`;
+exports[`sourceMap option should generate source maps when value is not specify and the "devtool" option has "source-map" value (node-sass) (scss): warnings 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "false" value (dart-sass) (sass): errors 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "false" value (dart-sass) (sass): warnings 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "false" value (dart-sass) (scss): errors 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "false" value (dart-sass) (scss): warnings 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "false" value (node-sass) (sass): errors 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "false" value (node-sass) (sass): warnings 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "false" value (node-sass) (scss): errors 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "false" value (node-sass) (scss): warnings 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "source-map" value (dart-sass) (sass): errors 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "source-map" value (dart-sass) (sass): warnings 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "source-map" value (dart-sass) (scss): errors 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "source-map" value (dart-sass) (scss): warnings 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "source-map" value (node-sass) (sass): errors 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "source-map" value (node-sass) (sass): warnings 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "source-map" value (node-sass) (scss): errors 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value has "false" and the "devtool" option has "source-map" value (node-sass) (scss): warnings 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value is not specify and the "devtool" option has "false" value (dart-sass) (sass): errors 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value is not specify and the "devtool" option has "false" value (dart-sass) (sass): warnings 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value is not specify and the "devtool" option has "false" value (dart-sass) (scss): errors 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value is not specify and the "devtool" option has "false" value (dart-sass) (scss): warnings 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value is not specify and the "devtool" option has "false" value (node-sass) (sass): errors 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value is not specify and the "devtool" option has "false" value (node-sass) (sass): warnings 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value is not specify and the "devtool" option has "false" value (node-sass) (scss): errors 1`] = `Array []`;
+
+exports[`sourceMap option should not generate source maps when value is not specify and the "devtool" option has "false" value (node-sass) (scss): warnings 1`] = `Array []`;
diff --git a/test/sourceMap-options.test.js b/test/sourceMap-options.test.js
@@ -22,12 +22,28 @@ describe('sourceMap option', () => {
     syntaxStyles.forEach((syntax) => {
       const [implementationName] = implementation.info.split('\t');
 
-      it(`not specify (${implementationName}) (${syntax})`, async () => {
+      it(`should generate source maps when value is not specify and the "devtool" option has "source-map" value (${implementationName}) (${syntax})`, async () => {
+        const testId = getTestId('language', syntax);
+        const options = {
+          implementation: getImplementationByName(implementationName),
+        };
+        const stats = await compile(testId, {
+          devtool: 'source-map',
+          loader: { options },
+        });
+        const { css, sourceMap } = getCodeFromBundle(stats);
+
+        expect(css).toBeDefined();
+        expect(sourceMap).toBeDefined();
+
+        expect(stats.compilation.warnings).toMatchSnapshot('warnings');
+        expect(stats.compilation.errors).toMatchSnapshot('errors');
+      });
+
+      it(`should not generate source maps when value is not specify and the "devtool" option has "false" value (${implementationName}) (${syntax})`, async () => {
         const testId = getTestId('language', syntax);
         const options = {
           implementation: getImplementationByName(implementationName),
-          // eslint-disable-next-line no-undefined
-          sourceMap: undefined,
         };
         const stats = await compile(testId, { loader: { options } });
         const { css, sourceMap } = getCodeFromBundle(stats);
@@ -39,7 +55,26 @@ describe('sourceMap option', () => {
         expect(stats.compilation.errors).toMatchSnapshot('errors');
       });
 
-      it(`false (${implementationName}) (${syntax})`, async () => {
+      it(`should not generate source maps when value has "false" and the "devtool" option has "source-map" value (${implementationName}) (${syntax})`, async () => {
+        const testId = getTestId('language', syntax);
+        const options = {
+          implementation: getImplementationByName(implementationName),
+          sourceMap: false,
+        };
+        const stats = await compile(testId, {
+          devtool: 'source-map',
+          loader: { options },
+        });
+        const { css, sourceMap } = getCodeFromBundle(stats);
+
+        expect(css).toBeDefined();
+        expect(sourceMap).toBeUndefined();
+
+        expect(stats.compilation.warnings).toMatchSnapshot('warnings');
+        expect(stats.compilation.errors).toMatchSnapshot('errors');
+      });
+
+      it(`should not generate source maps when value has "false" and the "devtool" option has "false" value (${implementationName}) (${syntax})`, async () => {
         const testId = getTestId('language', syntax);
         const options = {
           implementation: getImplementationByName(implementationName),
@@ -55,15 +90,50 @@ describe('sourceMap option', () => {
         expect(stats.compilation.errors).toMatchSnapshot('errors');
       });
 
-      it(`true (${implementationName}) (${syntax})`, async () => {
+      it(`should generate source maps when value has "true" value and the "devtool" option has "source-map" value (${implementationName}) (${syntax})`, async () => {
         expect.assertions(9);
 
         const testId = getTestId('language', syntax);
         const options = {
           implementation: getImplementationByName(implementationName),
           sourceMap: true,
         };
-        const stats = await compile(testId, { loader: { options } });
+        const stats = await compile(testId, {
+          devtool: 'source-map',
+          loader: { options },
+        });
+        const { css, sourceMap } = getCodeFromBundle(stats);
+
+        expect(css).toBeDefined();
+        expect(sourceMap).toBeDefined();
+
+        expect(sourceMap.file).toBeUndefined();
+        expect(sourceMap.sourceRoot).toBeDefined();
+
+        expect(sourceMap.sources).toHaveLength(2);
+
+        sourceMap.sources.forEach((sourcePath) => {
+          expect(
+            fs.existsSync(path.resolve(sourceMap.sourceRoot, sourcePath))
+          ).toBe(true);
+        });
+
+        expect(stats.compilation.warnings).toMatchSnapshot('warnings');
+        expect(stats.compilation.errors).toMatchSnapshot('errors');
+      });
+
+      it(`should generate source maps when value has "true" value and the "devtool" option has "false" value (${implementationName}) (${syntax})`, async () => {
+        expect.assertions(9);
+
+        const testId = getTestId('language', syntax);
+        const options = {
+          implementation: getImplementationByName(implementationName),
+          sourceMap: true,
+        };
+        const stats = await compile(testId, {
+          devtool: false,
+          loader: { options },
+        });
         const { css, sourceMap } = getCodeFromBundle(stats);
 
         expect(css).toBeDefined();