Skip to content

Commit

Permalink
fix: generate absolute sources for source maps (#882)
Browse files Browse the repository at this point in the history
BREAKING CHANGE: loader generates absolute `sources` in source maps, avoid modifying `sass` source maps if the `sourceMap` option is `false`
  • Loading branch information
evilebottnawi committed Aug 24, 2020
1 parent d2b532c commit 769a06e
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 18 deletions.
24 changes: 18 additions & 6 deletions src/index.js
Expand Up @@ -9,6 +9,7 @@ import {
getSassOptions,
getWebpackImporter,
getRenderFunctionFromSassImplementation,
absolutifySourceMapSource,
} from './utils';
import SassError from './SassError';

Expand All @@ -27,8 +28,15 @@ function loader(content) {
});

const implementation = getSassImplementation(options.implementation);
const sassOptions = getSassOptions(this, options, content, implementation);

const useSourceMap =
typeof options.sourceMap === 'boolean' ? options.sourceMap : this.sourceMap;
const sassOptions = getSassOptions(
this,
options,
content,
implementation,
useSourceMap
);
const shouldUseWebpackImporter =
typeof options.webpackImporter === 'boolean'
? options.webpackImporter
Expand Down Expand Up @@ -58,7 +66,8 @@ function loader(content) {
return;
}

if (result.map) {
// Modify source paths only for webpack, otherwise we do nothing
if (result.map && useSourceMap) {
// eslint-disable-next-line no-param-reassign
result.map = JSON.parse(result.map);

Expand All @@ -67,13 +76,16 @@ function loader(content) {
// eslint-disable-next-line no-param-reassign
delete result.map.file;

// eslint-disable-next-line no-param-reassign
result.sourceRoot = '';

// 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
// eslint-disable-next-line no-param-reassign
result.map.sourceRoot = path.normalize(result.map.sourceRoot);
// eslint-disable-next-line no-param-reassign
result.map.sources = result.map.sources.map(path.normalize);
result.map.sources = result.map.sources.map((source) =>
absolutifySourceMapSource(this.rootContext, source)
);
}

result.stats.includedFiles.forEach((includedFile) => {
Expand Down
47 changes: 38 additions & 9 deletions src/utils.js
Expand Up @@ -93,9 +93,16 @@ function proxyCustomImporters(importers, loaderContext) {
* @param {object} loaderOptions
* @param {string} content
* @param {object} implementation
* @param {boolean} useSourceMap
* @returns {Object}
*/
function getSassOptions(loaderContext, loaderOptions, content, implementation) {
function getSassOptions(
loaderContext,
loaderOptions,
content,
implementation,
useSourceMap
) {
const options = klona(
loaderOptions.sassOptions
? typeof loaderOptions.sassOptions === 'function'
Expand Down Expand Up @@ -143,23 +150,19 @@ function getSassOptions(loaderContext, loaderOptions, content, implementation) {
options.outputStyle = 'compressed';
}

const useSourceMap =
typeof loaderOptions.sourceMap === 'boolean'
? loaderOptions.sourceMap
: loaderContext.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.
// 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.css.map');
options.sourceMapRoot = process.cwd();
options.sourceMap = true;
options.outFile = path.join(loaderContext.rootContext, 'style.css.map');
// options.sourceMapRoot = process.cwd();
options.sourceMapContents = true;
options.omitSourceMapUrl = true;
options.sourceMapEmbed = false;
// options.sourceMapEmbed = false;
}

const { resourcePath } = loaderContext;
Expand Down Expand Up @@ -483,10 +486,36 @@ function getRenderFunctionFromSassImplementation(implementation) {
return nodeSassJobQueue.push.bind(nodeSassJobQueue);
}

const ABSOLUTE_SCHEME = /^[A-Za-z0-9+\-.]+:/;

function getURLType(source) {
if (source[0] === '/') {
if (source[1] === '/') {
return 'scheme-relative';
}

return 'path-absolute';
}

return ABSOLUTE_SCHEME.test(source) ? 'absolute' : 'path-relative';
}

function absolutifySourceMapSource(sourceRoot, source) {
const sourceType = getURLType(source);

// Do no touch `scheme-relative`, `path-absolute` and `absolute` types
if (sourceType === 'path-relative') {
return path.resolve(sourceRoot, path.normalize(source));
}

return source;
}

export {
getSassImplementation,
getSassOptions,
getWebpackResolver,
getWebpackImporter,
getRenderFunctionFromSassImplementation,
absolutifySourceMapSource,
};
12 changes: 9 additions & 3 deletions test/sourceMap-options.test.js
Expand Up @@ -42,7 +42,9 @@ describe('sourceMap option', () => {

sourceMap.sourceRoot = '';
sourceMap.sources = sourceMap.sources.map((source) =>
source.replace(/\\/g, '/')
path
.relative(path.resolve(__dirname, '..'), source)
.replace(/\\/g, '/')
);

expect(css).toMatchSnapshot('css');
Expand Down Expand Up @@ -124,7 +126,9 @@ describe('sourceMap option', () => {

sourceMap.sourceRoot = '';
sourceMap.sources = sourceMap.sources.map((source) =>
source.replace(/\\/g, '/')
path
.relative(path.resolve(__dirname, '..'), source)
.replace(/\\/g, '/')
);

expect(css).toMatchSnapshot('css');
Expand Down Expand Up @@ -157,7 +161,9 @@ describe('sourceMap option', () => {

sourceMap.sourceRoot = '';
sourceMap.sources = sourceMap.sources.map((source) =>
source.replace(/\\/g, '/')
path
.relative(path.resolve(__dirname, '..'), source)
.replace(/\\/g, '/')
);

expect(css).toMatchSnapshot('css');
Expand Down

0 comments on commit 769a06e

Please sign in to comment.