From d1df48bfcfc96e167fef9c88f49bfae7bc2aee22 Mon Sep 17 00:00:00 2001 From: cap-Bernardito Date: Wed, 20 May 2020 11:58:48 +0300 Subject: [PATCH 1/9] fix: absolute path for sources and add sources in dependendency --- src/index.js | 79 +++++++++++---------- src/{utils/flatten.js => utils.js} | 24 ++++++- test/__snapshots__/loader.test.js.snap | 44 ++++++++---- test/fixtures/charset-inline-source-map.txt | 1 + test/fixtures/indexed-sourcemap/file.js | 2 +- test/fixtures/indexed-sourcemap/file.js.map | 3 +- test/helpers/getCompiler.js | 30 +++++--- test/helpers/normalizeMap.js | 7 -- test/loader.test.js | 27 +++++++ 9 files changed, 144 insertions(+), 73 deletions(-) rename src/{utils/flatten.js => utils.js} (66%) create mode 100644 test/fixtures/charset-inline-source-map.txt diff --git a/src/index.js b/src/index.js index 303a9a9..da35b41 100644 --- a/src/index.js +++ b/src/index.js @@ -8,12 +8,13 @@ import path from 'path'; import validateOptions from 'schema-utils'; import async from 'neo-async'; import parseDataURL from 'data-urls'; +import sourceMap from 'source-map'; import { labelToName, decode } from 'whatwg-encoding'; import { getOptions, urlToRequest } from 'loader-utils'; - -import flattenSourceMap from './utils/flatten'; +import { isAbsolute } from 'source-map/lib/util'; import schema from './options.json'; +import { flattenSourceMap, readFile, normalize } from './utils'; // Matches only the last occurrence of sourceMappingURL const baseRegex = @@ -121,43 +122,35 @@ export default function loader(input, inputMap) { map = await flattenSourceMap(map); } - if (map.sourcesContent && map.sourcesContent.length >= map.sources.length) { - callback(null, input.replace(match[0], ''), map); - - return; - } - - const sourcePrefix = map.sourceRoot ? `${map.sourceRoot}${path.sep}` : ''; - - // eslint-disable-next-line no-param-reassign - map.sources = map.sources.map((s) => sourcePrefix + s); - - // eslint-disable-next-line no-param-reassign - delete map.sourceRoot; - - const missingSources = map.sourcesContent - ? map.sources.slice(map.sourcesContent.length) - : map.sources; + const originalMap = await new sourceMap.SourceMapConsumer(map); async.map( - missingSources, + map.sources, // eslint-disable-next-line no-shadow (source, callback) => { - resolve(context, urlToRequest(source, true), (resolveError, result) => { - if (resolveError) { - emitWarning(`Cannot find source file '${source}': ${resolveError}`); + const fullPath = map.sourceRoot + ? `${map.sourceRoot}/${source}` + : source; - callback(null, null); + if (isAbsolute(fullPath)) { + const sourceContent = originalMap.sourceContentFor(source, true); + if (sourceContent) { + callback(null, { source: fullPath, content: sourceContent }); return; } - addDependency(result); + readFile(fullPath, 'utf-8', callback, emitWarning); + return; + } - fs.readFile(result, 'utf-8', (readFileError, content) => { - if (readFileError) { + resolve( + context, + urlToRequest(fullPath, true), + (resolveError, result) => { + if (resolveError) { emitWarning( - `Cannot open source file '${result}': ${readFileError}` + `Cannot find source file '${source}': ${resolveError}` ); callback(null, null); @@ -165,25 +158,35 @@ export default function loader(input, inputMap) { return; } - callback(null, { source: result, content }); - }); - }); + readFile(result, 'utf-8', callback, emitWarning); + } + ); }, (err, info) => { - // eslint-disable-next-line no-param-reassign - map.sourcesContent = map.sourcesContent || []; + const resultMap = { ...map }; + resultMap.sources = []; + resultMap.sourcesContent = []; + + delete resultMap.sourceRoot; info.forEach((res) => { if (res) { // eslint-disable-next-line no-param-reassign - map.sources[map.sourcesContent.length] = res.source; - map.sourcesContent.push(res.content); - } else { - map.sourcesContent.push(null); + resultMap.sources.push(normalize(res.source)); + resultMap.sourcesContent.push(res.content); + + if (res.source) { + addDependency(res.source); + } } }); - processMap(map, context, callback); + if (resultMap.sources.length === 0) { + callback(null, input, map); + return; + } + + callback(null, input.replace(match[0], ''), resultMap); } ); } diff --git a/src/utils/flatten.js b/src/utils.js similarity index 66% rename from src/utils/flatten.js rename to src/utils.js index 0003eff..61471ba 100644 --- a/src/utils/flatten.js +++ b/src/utils.js @@ -1,6 +1,8 @@ +import fs from 'fs'; + import sourceMap from 'source-map'; -async function FlattenSourceMap(map) { +async function flattenSourceMap(map) { const consumer = await new sourceMap.SourceMapConsumer(map); let generatedMap; @@ -43,4 +45,22 @@ async function FlattenSourceMap(map) { return generatedMap.toJSON(); } -module.exports = FlattenSourceMap; +function normalize(path) { + return path.replace(/\\/g, '/'); +} + +function readFile(fullPath, charset, callback, emitWarning) { + fs.readFile(fullPath, charset, (readFileError, content) => { + if (readFileError) { + emitWarning(`Cannot open source file '${fullPath}': ${readFileError}`); + + callback(null, null); + + return; + } + + callback(null, { source: fullPath, content }); + }); +} + +export { flattenSourceMap, readFile, normalize }; diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap index 78dce75..519bb3d 100644 --- a/test/__snapshots__/loader.test.js.snap +++ b/test/__snapshots__/loader.test.js.snap @@ -31,6 +31,7 @@ exports[`source-map-loader should process external SourceMaps (external sources) exports[`source-map-loader should process external SourceMaps: css 1`] = ` "with SourceMap +//#sourceMappingURL=external-source-map.map // comment" `; @@ -50,7 +51,12 @@ Object { } `; -exports[`source-map-loader should process external SourceMaps: warnings 1`] = `Array []`; +exports[`source-map-loader should process external SourceMaps: warnings 1`] = ` +Array [ + "ModuleWarning: Module Warning (from \`replaced original path\`): +(Emitted value instead of an instance of Error) Cannot find source file 'external-source-map.txt': Error: Can't resolve './external-source-map.txt' in '/test/fixtures'", +] +`; exports[`source-map-loader should process inlined SourceMaps with charset: css 1`] = ` "with SourceMap @@ -64,10 +70,11 @@ Object { "file": "charset-inline-source-map.js", "mappings": "AAAA", "sources": Array [ - "charset-inline-source-map.txt", + "/test/fixtures/charset-inline-source-map.txt - (normalized for test)", ], "sourcesContent": Array [ - "with SourceMap", + "// Content +", ], "version": 3, } @@ -77,6 +84,7 @@ exports[`source-map-loader should process inlined SourceMaps with charset: warni exports[`source-map-loader should process inlined SourceMaps: css 1`] = ` "with SourceMap +// @ sourceMappingURL = data:application/source-map;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiaW5saW5lLXNvdXJjZS1tYXAuanMiLCJzb3VyY2VzIjpbImlubGluZS1zb3VyY2UtbWFwLnR4dCJdLCJzb3VyY2VzQ29udGVudCI6WyJ3aXRoIFNvdXJjZU1hcCJdLCJtYXBwaW5ncyI6IkFBQUEifQ== // comment" `; @@ -96,7 +104,12 @@ Object { } `; -exports[`source-map-loader should process inlined SourceMaps: warnings 1`] = `Array []`; +exports[`source-map-loader should process inlined SourceMaps: warnings 1`] = ` +Array [ + "ModuleWarning: Module Warning (from \`replaced original path\`): +(Emitted value instead of an instance of Error) Cannot find source file 'inline-source-map.txt': Error: Can't resolve './inline-source-map.txt' in '/test/fixtures'", +] +`; exports[`source-map-loader should skip invalid base64 SourceMap: css 1`] = ` "without SourceMap @@ -133,7 +146,7 @@ Object { exports[`source-map-loader should support absolute sourceRoot paths in sourcemaps: warnings 1`] = `Array []`; exports[`source-map-loader should support indexed sourcemaps: css 1`] = ` -"with SourceMap +"console.log('with SourceMap') // Map taken from here // https://github.com/mozilla/source-map/blob/master/test/util.js - indexedTestMapDifferentSourceRoots " @@ -147,13 +160,14 @@ Object { "mappings": "CAAC,IAAI,IAAM,SAAU,GAClB,OAAO,IAAI;CCDb,IAAI,IAAM,SAAU,GAClB,OAAO", "names": Array [], "sources": Array [ - "/the/root/nested1.js", + "/test/fixtures/indexed-sourcemap/nested1.js - (normalized for test)", "/different/root/nested2.js", ], "sourcesContent": Array [ - " ONE.foo = function (bar) { - return baz(bar); - };", + "ONE.foo = function (bar) { + return baz(bar); +}; +", " TWO.inc = function (n) { return n + 1; };", @@ -191,6 +205,7 @@ exports[`source-map-loader should support relative sourceRoot paths in sourcemap exports[`source-map-loader should use last SourceMap directive: css 1`] = ` "with SourceMap anInvalidDirective = \\"\\\\n/*# sourceMappingURL=data:application/json;base64,\\"+btoa(unescape(encodeURIComponent(JSON.stringify(sourceMap))))+\\" */\\"; +// @ sourceMappingURL = data:application/source-map;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiaW5saW5lLXNvdXJjZS1tYXAuanMiLCJzb3VyY2VzIjpbImlubGluZS1zb3VyY2UtbWFwLnR4dCJdLCJzb3VyY2VzQ29udGVudCI6WyJ3aXRoIFNvdXJjZU1hcCJdLCJtYXBwaW5ncyI6IkFBQUEifQ== // comment" `; @@ -210,7 +225,12 @@ Object { } `; -exports[`source-map-loader should use last SourceMap directive: warnings 1`] = `Array []`; +exports[`source-map-loader should use last SourceMap directive: warnings 1`] = ` +Array [ + "ModuleWarning: Module Warning (from \`replaced original path\`): +(Emitted value instead of an instance of Error) Cannot find source file 'inline-source-map.txt': Error: Can't resolve './inline-source-map.txt' in '/test/fixtures'", +] +`; exports[`source-map-loader should warn on invalid SourceMap: css 1`] = ` "with SourceMap @@ -259,6 +279,7 @@ Array [ exports[`source-map-loader should warn on missing source file: css 1`] = ` "with SourceMap +//#sourceMappingURL=missing-source-map2.map // comment" `; @@ -271,9 +292,6 @@ Object { "sources": Array [ "missing-source-map2.txt", ], - "sourcesContent": Array [ - null, - ], "version": 3, } `; diff --git a/test/fixtures/charset-inline-source-map.txt b/test/fixtures/charset-inline-source-map.txt new file mode 100644 index 0000000..c5b4408 --- /dev/null +++ b/test/fixtures/charset-inline-source-map.txt @@ -0,0 +1 @@ +// Content diff --git a/test/fixtures/indexed-sourcemap/file.js b/test/fixtures/indexed-sourcemap/file.js index 2be7348..83076fd 100644 --- a/test/fixtures/indexed-sourcemap/file.js +++ b/test/fixtures/indexed-sourcemap/file.js @@ -1,4 +1,4 @@ -with SourceMap +console.log('with SourceMap') //#sourceMappingURL=file.js.map // Map taken from here // https://github.com/mozilla/source-map/blob/master/test/util.js - indexedTestMapDifferentSourceRoots diff --git a/test/fixtures/indexed-sourcemap/file.js.map b/test/fixtures/indexed-sourcemap/file.js.map index 1dc44df..930cc52 100644 --- a/test/fixtures/indexed-sourcemap/file.js.map +++ b/test/fixtures/indexed-sourcemap/file.js.map @@ -15,8 +15,7 @@ ], "names": ["bar", "baz"], "mappings": "CAAC,IAAI,IAAM,SAAUA,GAClB,OAAOC,IAAID", - "file": "file.js", - "sourceRoot": "/the/root" + "file": "file.js" } }, { diff --git a/test/helpers/getCompiler.js b/test/helpers/getCompiler.js index b391311..6e34d5a 100644 --- a/test/helpers/getCompiler.js +++ b/test/helpers/getCompiler.js @@ -3,7 +3,25 @@ import path from 'path'; import webpack from 'webpack'; import { createFsFromVolume, Volume } from 'memfs'; -export default (fixture, loaderOptions = {}, config = {}) => { +export default ( + fixture, + loaderOptions = {}, + config = {}, + skipTestLoader = false +) => { + const loaders = [ + { + loader: path.resolve(__dirname, '../../src'), + options: loaderOptions || {}, + }, + ]; + + if (!skipTestLoader) { + loaders.unshift({ + loader: require.resolve('./testLoader'), + }); + } + const fullConfig = { mode: 'development', devtool: config.devtool || 'source-map', @@ -19,15 +37,7 @@ export default (fixture, loaderOptions = {}, config = {}) => { rules: [ { test: /\.js/i, - rules: [ - { - loader: require.resolve('./testLoader'), - }, - { - loader: path.resolve(__dirname, '../../src'), - options: loaderOptions || {}, - }, - ], + use: loaders, }, ], }, diff --git a/test/helpers/normalizeMap.js b/test/helpers/normalizeMap.js index 89c0bf9..bc5b279 100644 --- a/test/helpers/normalizeMap.js +++ b/test/helpers/normalizeMap.js @@ -33,13 +33,6 @@ function removeCWD(str) { let cwd = process.cwd(); if (isWin) { - // Todo: explore the issue - // if (str.includes('/')) { - // throw new Error( - // 'There should not be a forward slash in the Windows path' - // ); - // } - // eslint-disable-next-line no-param-reassign str = str.replace(/\\/g, '/'); cwd = cwd.replace(/\\/g, '/'); diff --git a/test/loader.test.js b/test/loader.test.js index e2261bf..04d21c3 100644 --- a/test/loader.test.js +++ b/test/loader.test.js @@ -8,6 +8,7 @@ import { getErrors, normalizeMap, getWarnings, + readAsset, } from './helpers'; describe('source-map-loader', () => { @@ -267,6 +268,8 @@ describe('source-map-loader', () => { const dependencies = [ path.join(currentDirPath, 'file.js'), path.join(currentDirPath, 'file.js.map'), + path.join(currentDirPath, 'nested1.js'), + `/different/root/nested2.js`, ]; dependencies.forEach((fixture) => { @@ -278,4 +281,28 @@ describe('source-map-loader', () => { expect(getWarnings(stats)).toMatchSnapshot('warnings'); expect(getErrors(stats)).toMatchSnapshot('errors'); }); + + it('should transform to webpack', async () => { + const currentDirPath = path.join( + __dirname, + 'fixtures', + 'indexed-sourcemap' + ); + + const testId = path.join(currentDirPath, 'file.js'); + const compiler = getCompiler(testId, {}, {}, true); + const stats = await compile(compiler); + const bundle = readAsset('main.bundle.js.map', compiler, stats); + + const dependencies = [ + 'indexed-sourcemap/nested1.js', + 'different/root/nested2.js', + 'webpack/bootstrap', + ]; + + // Todo: rewrite when we will fix issue whith unresolved paths + dependencies.forEach((fixture) => { + expect(bundle.indexOf(fixture) !== -1).toBe(true); + }); + }); }); From 3577979851d8e82c0517bc1de52a4b37e3fb0cb6 Mon Sep 17 00:00:00 2001 From: cap-Bernardito Date: Wed, 20 May 2020 19:35:30 +0300 Subject: [PATCH 2/9] fix: absolute path for sources and add sources in dependendency --- test/fixtures/charset-inline-source-map.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/charset-inline-source-map.txt b/test/fixtures/charset-inline-source-map.txt index c5b4408..0bc300d 100644 --- a/test/fixtures/charset-inline-source-map.txt +++ b/test/fixtures/charset-inline-source-map.txt @@ -1 +1 @@ -// Content +// Content From beda794d6629c67bbfabdac38169c892c0e1cbb0 Mon Sep 17 00:00:00 2001 From: cap-Bernardito Date: Wed, 20 May 2020 19:38:46 +0300 Subject: [PATCH 3/9] fix: absolute path for sources and add sources in dependendency --- test/fixtures/charset-inline-source-map.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/charset-inline-source-map.txt b/test/fixtures/charset-inline-source-map.txt index 0bc300d..c5b4408 100644 --- a/test/fixtures/charset-inline-source-map.txt +++ b/test/fixtures/charset-inline-source-map.txt @@ -1 +1 @@ -// Content +// Content From 32f15337522ac6c106215dcdcf7782f1aec5ef91 Mon Sep 17 00:00:00 2001 From: cap-Bernardito Date: Thu, 21 May 2020 14:01:08 +0300 Subject: [PATCH 4/9] fix: absolute path for sources and add sources in dependendency --- package-lock.json | 3 +- package.json | 1 - src/index.js | 107 ++++++++++++------------- src/utils.js | 44 +++++++--- test/__snapshots__/loader.test.js.snap | 14 +--- 5 files changed, 91 insertions(+), 78 deletions(-) diff --git a/package-lock.json b/package-lock.json index 676374a..79a83e1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12069,7 +12069,8 @@ "neo-async": { "version": "2.6.1", "resolved": "https://registry.npmjs.org/neo-async/-/neo-async-2.6.1.tgz", - "integrity": "sha512-iyam8fBuCUpWeKPGpaNMetEocMt364qkCsfL9JuhjXX6dRnguRVOfk2GZaDpPjcOKiiXCPINZC1GczQ7iTq3Zw==" + "integrity": "sha512-iyam8fBuCUpWeKPGpaNMetEocMt364qkCsfL9JuhjXX6dRnguRVOfk2GZaDpPjcOKiiXCPINZC1GczQ7iTq3Zw==", + "dev": true }, "nice-try": { "version": "1.0.5", diff --git a/package.json b/package.json index 0572eaf..d6ba112 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,6 @@ "dependencies": { "data-urls": "^2.0.0", "loader-utils": "^2.0.0", - "neo-async": "^2.6.1", "schema-utils": "^2.6.6", "source-map": "^0.6.0", "whatwg-encoding": "^1.0.5" diff --git a/src/index.js b/src/index.js index da35b41..b2e78b5 100644 --- a/src/index.js +++ b/src/index.js @@ -6,15 +6,14 @@ import fs from 'fs'; import path from 'path'; import validateOptions from 'schema-utils'; -import async from 'neo-async'; import parseDataURL from 'data-urls'; -import sourceMap from 'source-map'; +import { SourceMapConsumer } from 'source-map'; import { labelToName, decode } from 'whatwg-encoding'; import { getOptions, urlToRequest } from 'loader-utils'; import { isAbsolute } from 'source-map/lib/util'; import schema from './options.json'; -import { flattenSourceMap, readFile, normalize } from './utils'; +import { flattenSourceMap, normalize, MapAgregator } from './utils'; // Matches only the last occurrence of sourceMappingURL const baseRegex = @@ -122,72 +121,68 @@ export default function loader(input, inputMap) { map = await flattenSourceMap(map); } - const originalMap = await new sourceMap.SourceMapConsumer(map); + const mapConsumer = await new SourceMapConsumer(map); - async.map( - map.sources, - // eslint-disable-next-line no-shadow - (source, callback) => { + const resolvedSources = await Promise.all( + map.sources.map(async (source) => { const fullPath = map.sourceRoot ? `${map.sourceRoot}/${source}` : source; + const sourceData = new MapAgregator({ + mapConsumer, + source, + fullPath, + emitWarning, + }); if (isAbsolute(fullPath)) { - const sourceContent = originalMap.sourceContentFor(source, true); + return sourceData.content; + } - if (sourceContent) { - callback(null, { source: fullPath, content: sourceContent }); - return; - } + return new Promise((promiseResolve) => { + resolve( + context, + urlToRequest(fullPath, true), + (resolveError, result) => { + if (resolveError) { + emitWarning( + `Cannot find source file '${source}': ${resolveError}` + ); + + return promiseResolve(sourceData.placeholderContent); + } + + sourceData.setFullPath(result); + return promiseResolve(sourceData.content); + } + ); + }); + }) + ); - readFile(fullPath, 'utf-8', callback, emitWarning); - return; - } + const resultMap = { ...map }; + resultMap.sources = []; + resultMap.sourcesContent = []; - resolve( - context, - urlToRequest(fullPath, true), - (resolveError, result) => { - if (resolveError) { - emitWarning( - `Cannot find source file '${source}': ${resolveError}` - ); + delete resultMap.sourceRoot; - callback(null, null); + resolvedSources.forEach((res) => { + // eslint-disable-next-line no-param-reassign + resultMap.sources.push(normalize(res.source)); + resultMap.sourcesContent.push(res.content); - return; - } + if (res.source) { + addDependency(res.source); + } + }); - readFile(result, 'utf-8', callback, emitWarning); - } - ); - }, - (err, info) => { - const resultMap = { ...map }; - resultMap.sources = []; - resultMap.sourcesContent = []; - - delete resultMap.sourceRoot; - - info.forEach((res) => { - if (res) { - // eslint-disable-next-line no-param-reassign - resultMap.sources.push(normalize(res.source)); - resultMap.sourcesContent.push(res.content); - - if (res.source) { - addDependency(res.source); - } - } - }); + const sourcesContentIsEmpty = + resultMap.sourcesContent.filter((entry) => !!entry).length === 0; - if (resultMap.sources.length === 0) { - callback(null, input, map); - return; - } + if (sourcesContentIsEmpty) { + delete resultMap.sourcesContent; + } - callback(null, input.replace(match[0], ''), resultMap); - } - ); + callback(null, input.replace(match[0], ''), resultMap); } } diff --git a/src/utils.js b/src/utils.js index 61471ba..af3a43b 100644 --- a/src/utils.js +++ b/src/utils.js @@ -49,18 +49,42 @@ function normalize(path) { return path.replace(/\\/g, '/'); } -function readFile(fullPath, charset, callback, emitWarning) { - fs.readFile(fullPath, charset, (readFileError, content) => { - if (readFileError) { - emitWarning(`Cannot open source file '${fullPath}': ${readFileError}`); +function readFile(fullPath, charset, emitWarning) { + return new Promise((resolve) => { + fs.readFile(fullPath, charset, (readFileError, content) => { + if (readFileError) { + emitWarning(`Cannot open source file '${fullPath}': ${readFileError}`); - callback(null, null); + resolve({ source: fullPath, content: null }); + } - return; - } - - callback(null, { source: fullPath, content }); + resolve({ source: fullPath, content }); + }); }); } -export { flattenSourceMap, readFile, normalize }; +class MapAgregator { + constructor({ mapConsumer, source, fullPath, emitWarning }) { + this.fullPath = fullPath; + this.sourceContent = mapConsumer.sourceContentFor(source, true); + this.emitWarning = emitWarning; + } + + setFullPath(path) { + this.fullPath = path; + } + + get content() { + return this.sourceContent + ? { source: this.fullPath, content: this.sourceContent } + : readFile(this.fullPath, 'utf-8', this.emitWarning); + } + + get placeholderContent() { + return this.sourceContent + ? { source: this.fullPath, content: this.sourceContent } + : { source: this.fullPath, content: null }; + } +} + +export { flattenSourceMap, normalize, MapAgregator }; diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap index 519bb3d..0f9f84f 100644 --- a/test/__snapshots__/loader.test.js.snap +++ b/test/__snapshots__/loader.test.js.snap @@ -31,7 +31,6 @@ exports[`source-map-loader should process external SourceMaps (external sources) exports[`source-map-loader should process external SourceMaps: css 1`] = ` "with SourceMap -//#sourceMappingURL=external-source-map.map // comment" `; @@ -73,8 +72,7 @@ Object { "/test/fixtures/charset-inline-source-map.txt - (normalized for test)", ], "sourcesContent": Array [ - "// Content -", + "with SourceMap", ], "version": 3, } @@ -84,7 +82,6 @@ exports[`source-map-loader should process inlined SourceMaps with charset: warni exports[`source-map-loader should process inlined SourceMaps: css 1`] = ` "with SourceMap -// @ sourceMappingURL = data:application/source-map;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiaW5saW5lLXNvdXJjZS1tYXAuanMiLCJzb3VyY2VzIjpbImlubGluZS1zb3VyY2UtbWFwLnR4dCJdLCJzb3VyY2VzQ29udGVudCI6WyJ3aXRoIFNvdXJjZU1hcCJdLCJtYXBwaW5ncyI6IkFBQUEifQ== // comment" `; @@ -164,10 +161,9 @@ Object { "/different/root/nested2.js", ], "sourcesContent": Array [ - "ONE.foo = function (bar) { - return baz(bar); -}; -", + " ONE.foo = function (bar) { + return baz(bar); + };", " TWO.inc = function (n) { return n + 1; };", @@ -205,7 +201,6 @@ exports[`source-map-loader should support relative sourceRoot paths in sourcemap exports[`source-map-loader should use last SourceMap directive: css 1`] = ` "with SourceMap anInvalidDirective = \\"\\\\n/*# sourceMappingURL=data:application/json;base64,\\"+btoa(unescape(encodeURIComponent(JSON.stringify(sourceMap))))+\\" */\\"; -// @ sourceMappingURL = data:application/source-map;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiaW5saW5lLXNvdXJjZS1tYXAuanMiLCJzb3VyY2VzIjpbImlubGluZS1zb3VyY2UtbWFwLnR4dCJdLCJzb3VyY2VzQ29udGVudCI6WyJ3aXRoIFNvdXJjZU1hcCJdLCJtYXBwaW5ncyI6IkFBQUEifQ== // comment" `; @@ -279,7 +274,6 @@ Array [ exports[`source-map-loader should warn on missing source file: css 1`] = ` "with SourceMap -//#sourceMappingURL=missing-source-map2.map // comment" `; From 7e88097e4f9f0db8a44f759410f72dbaf6005b5d Mon Sep 17 00:00:00 2001 From: cap-Bernardito Date: Thu, 21 May 2020 18:09:21 +0300 Subject: [PATCH 5/9] fix: absolute path for sources and add sources in dependendency --- src/index.js | 112 +++++++++++++++++++++++------------ src/utils.js | 32 ++-------- test/helpers/getCompiler.js | 1 + test/helpers/normalizeMap.js | 4 ++ 4 files changed, 83 insertions(+), 66 deletions(-) diff --git a/src/index.js b/src/index.js index b2e78b5..148b334 100644 --- a/src/index.js +++ b/src/index.js @@ -4,16 +4,21 @@ */ import fs from 'fs'; import path from 'path'; +import urlUtils from 'url'; import validateOptions from 'schema-utils'; import parseDataURL from 'data-urls'; + import { SourceMapConsumer } from 'source-map'; import { labelToName, decode } from 'whatwg-encoding'; import { getOptions, urlToRequest } from 'loader-utils'; -import { isAbsolute } from 'source-map/lib/util'; import schema from './options.json'; -import { flattenSourceMap, normalize, MapAgregator } from './utils'; +import { + flattenSourceMap, + readFile, + getContentFromSourcesContent, +} from './utils'; // Matches only the last occurrence of sourceMappingURL const baseRegex = @@ -22,6 +27,7 @@ const baseRegex = const regex1 = new RegExp(`/\\*${baseRegex}\\s*\\*/`); // Matches // .... comments const regex2 = new RegExp(`//${baseRegex}($|\n|\r\n?)`); +const notProcessedProtocols = ['http:', 'https:', 'ftp:']; export default function loader(input, inputMap) { const options = getOptions(this); @@ -40,7 +46,7 @@ export default function loader(input, inputMap) { return; } - const [, url] = match; + let [, url] = match; const dataURL = parseDataURL(url); @@ -78,6 +84,21 @@ export default function loader(input, inputMap) { return; } + const parsedUrl = urlUtils.parse(url); + const { protocol } = parsedUrl; + + if (protocol === 'file:') { + url = urlUtils.fileURLToPath(url); + } + + if (notProcessedProtocols.includes(protocol)) { + emitWarning(`URL scheme not supported: ${protocol}`); + + callback(null, input, inputMap); + + return; + } + resolve(context, urlToRequest(url, true), (resolveError, result) => { if (resolveError) { emitWarning(`Cannot find SourceMap '${url}': ${resolveError}`); @@ -123,42 +144,57 @@ export default function loader(input, inputMap) { const mapConsumer = await new SourceMapConsumer(map); - const resolvedSources = await Promise.all( - map.sources.map(async (source) => { - const fullPath = map.sourceRoot - ? `${map.sourceRoot}/${source}` - : source; - const sourceData = new MapAgregator({ - mapConsumer, - source, - fullPath, - emitWarning, - }); - - if (isAbsolute(fullPath)) { - return sourceData.content; - } - - return new Promise((promiseResolve) => { - resolve( - context, - urlToRequest(fullPath, true), - (resolveError, result) => { - if (resolveError) { - emitWarning( - `Cannot find source file '${source}': ${resolveError}` - ); - - return promiseResolve(sourceData.placeholderContent); - } + let resolvedSources; - sourceData.setFullPath(result); - return promiseResolve(sourceData.content); - } + try { + resolvedSources = await Promise.all( + map.sources.map(async (source) => { + const fullPath = map.sourceRoot + ? `${map.sourceRoot}${path.sep}${source}` + : source; + + const originalData = getContentFromSourcesContent( + mapConsumer, + source ); - }); - }) - ); + + if (path.isAbsolute(fullPath)) { + return originalData + ? { source: fullPath, content: originalData } + : readFile(fullPath, 'utf-8', emitWarning); + } + + return new Promise((promiseResolve) => { + resolve( + context, + urlToRequest(fullPath, true), + (resolveError, result) => { + if (resolveError) { + emitWarning( + `Cannot find source file '${source}': ${resolveError}` + ); + + return originalData + ? promiseResolve({ + source: fullPath, + content: originalData, + }) + : promiseResolve({ source: fullPath, content: null }); + } + + return originalData + ? promiseResolve({ source: result, content: originalData }) + : promiseResolve(readFile(result, 'utf-8', emitWarning)); + } + ); + }); + }) + ); + } catch (error) { + emitWarning(error); + + callback(null, input, inputMap); + } const resultMap = { ...map }; resultMap.sources = []; @@ -168,7 +204,7 @@ export default function loader(input, inputMap) { resolvedSources.forEach((res) => { // eslint-disable-next-line no-param-reassign - resultMap.sources.push(normalize(res.source)); + resultMap.sources.push(path.normalize(res.source)); resultMap.sourcesContent.push(res.content); if (res.source) { diff --git a/src/utils.js b/src/utils.js index af3a43b..2f37f92 100644 --- a/src/utils.js +++ b/src/utils.js @@ -45,17 +45,13 @@ async function flattenSourceMap(map) { return generatedMap.toJSON(); } -function normalize(path) { - return path.replace(/\\/g, '/'); -} - function readFile(fullPath, charset, emitWarning) { return new Promise((resolve) => { fs.readFile(fullPath, charset, (readFileError, content) => { if (readFileError) { emitWarning(`Cannot open source file '${fullPath}': ${readFileError}`); - resolve({ source: fullPath, content: null }); + resolve({ source: null, content: null }); } resolve({ source: fullPath, content }); @@ -63,28 +59,8 @@ function readFile(fullPath, charset, emitWarning) { }); } -class MapAgregator { - constructor({ mapConsumer, source, fullPath, emitWarning }) { - this.fullPath = fullPath; - this.sourceContent = mapConsumer.sourceContentFor(source, true); - this.emitWarning = emitWarning; - } - - setFullPath(path) { - this.fullPath = path; - } - - get content() { - return this.sourceContent - ? { source: this.fullPath, content: this.sourceContent } - : readFile(this.fullPath, 'utf-8', this.emitWarning); - } - - get placeholderContent() { - return this.sourceContent - ? { source: this.fullPath, content: this.sourceContent } - : { source: this.fullPath, content: null }; - } +function getContentFromSourcesContent(consumer, source) { + return consumer.sourceContentFor(source, true); } -export { flattenSourceMap, normalize, MapAgregator }; +export { flattenSourceMap, readFile, getContentFromSourcesContent }; diff --git a/test/helpers/getCompiler.js b/test/helpers/getCompiler.js index 6e34d5a..c816e2b 100644 --- a/test/helpers/getCompiler.js +++ b/test/helpers/getCompiler.js @@ -32,6 +32,7 @@ export default ( filename: '[name].bundle.js', chunkFilename: '[name].chunk.js', library: 'sourceMapLoaderExport', + // devtoolModuleFilenameTemplate: "[absolute-resource-path]" }, module: { rules: [ diff --git a/test/helpers/normalizeMap.js b/test/helpers/normalizeMap.js index bc5b279..0c5951b 100644 --- a/test/helpers/normalizeMap.js +++ b/test/helpers/normalizeMap.js @@ -24,6 +24,10 @@ function normilizeArr(arr) { return str; } + if (str.replace(/\\/g, '/') === normilized) { + return normilized; + } + return `${normilized} - (normalized for test)`; }); } From e293c195f0d7901bb8e4ec78fae8c8be0e3d609b Mon Sep 17 00:00:00 2001 From: cap-Bernardito Date: Thu, 21 May 2020 18:31:25 +0300 Subject: [PATCH 6/9] fix: absolute path for sources and add sources in dependendency --- test/loader.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/loader.test.js b/test/loader.test.js index 04d21c3..8500a36 100644 --- a/test/loader.test.js +++ b/test/loader.test.js @@ -296,7 +296,7 @@ describe('source-map-loader', () => { const dependencies = [ 'indexed-sourcemap/nested1.js', - 'different/root/nested2.js', + 'nested2.js', 'webpack/bootstrap', ]; From 0d02e3902820e2be01c2c488fcab9d71c2473487 Mon Sep 17 00:00:00 2001 From: cap-Bernardito Date: Thu, 21 May 2020 20:09:32 +0300 Subject: [PATCH 7/9] fix: absolute path for sources and add sources in dependendency --- src/index.js | 27 ++++++++++++++-------- src/utils.js | 24 ++++++++++++++++++- test/__snapshots__/loader.test.js.snap | 32 ++++++++++++++++++++++++++ test/fixtures/file-source-map.js | 3 +++ test/fixtures/http-source-map.js | 3 +++ test/loader.test.js | 22 ++++++++++++++++++ 6 files changed, 100 insertions(+), 11 deletions(-) create mode 100644 test/fixtures/file-source-map.js create mode 100644 test/fixtures/http-source-map.js diff --git a/src/index.js b/src/index.js index 148b334..0debb6a 100644 --- a/src/index.js +++ b/src/index.js @@ -18,6 +18,7 @@ import { flattenSourceMap, readFile, getContentFromSourcesContent, + isUrlRequest, } from './utils'; // Matches only the last occurrence of sourceMappingURL @@ -27,7 +28,6 @@ const baseRegex = const regex1 = new RegExp(`/\\*${baseRegex}\\s*\\*/`); // Matches // .... comments const regex2 = new RegExp(`//${baseRegex}($|\n|\r\n?)`); -const notProcessedProtocols = ['http:', 'https:', 'ftp:']; export default function loader(input, inputMap) { const options = getOptions(this); @@ -84,19 +84,26 @@ export default function loader(input, inputMap) { return; } - const parsedUrl = urlUtils.parse(url); - const { protocol } = parsedUrl; + if (!isUrlRequest(url)) { + const { protocol } = urlUtils.parse(url); - if (protocol === 'file:') { - url = urlUtils.fileURLToPath(url); - } + if (protocol !== 'file:') { + emitWarning(`URL scheme not supported: ${protocol}`); - if (notProcessedProtocols.includes(protocol)) { - emitWarning(`URL scheme not supported: ${protocol}`); + callback(null, input, inputMap); - callback(null, input, inputMap); + return; + } - return; + try { + url = urlUtils.fileURLToPath(url); + } catch (error) { + emitWarning(error); + + callback(null, input, inputMap); + + return; + } } resolve(context, urlToRequest(url, true), (resolveError, result) => { diff --git a/src/utils.js b/src/utils.js index 2f37f92..b3ff87d 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,4 +1,5 @@ import fs from 'fs'; +import path from 'path'; import sourceMap from 'source-map'; @@ -63,4 +64,25 @@ function getContentFromSourcesContent(consumer, source) { return consumer.sourceContentFor(source, true); } -export { flattenSourceMap, readFile, getContentFromSourcesContent }; +function isUrlRequest(url) { + // An URL is not an request if + + // 1. It's an absolute url and it is not `windows` path like `C:\dir\file` + if (/^[a-z][a-z0-9+.-]*:/i.test(url) && !path.win32.isAbsolute(url)) { + return false; + } + + // 2. It's a protocol-relative + if (/^\/\//.test(url)) { + return false; + } + + return true; +} + +export { + flattenSourceMap, + readFile, + getContentFromSourcesContent, + isUrlRequest, +}; diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap index 0f9f84f..ad2bdbd 100644 --- a/test/__snapshots__/loader.test.js.snap +++ b/test/__snapshots__/loader.test.js.snap @@ -108,6 +108,38 @@ Array [ ] `; +exports[`source-map-loader should reject http SourceMaps: css 1`] = ` +"with SourceMap +//#sourceMappingURL=http://sampledomain.com/external-source-map2.map +// comment +" +`; + +exports[`source-map-loader should reject http SourceMaps: errors 1`] = `Array []`; + +exports[`source-map-loader should reject http SourceMaps: warnings 1`] = ` +Array [ + "ModuleWarning: Module Warning (from \`replaced original path\`): +(Emitted value instead of an instance of Error) URL scheme not supported: http:", +] +`; + +exports[`source-map-loader should reject not valid file: SourceMaps: css 1`] = ` +"with SourceMap +//#sourceMappingURL=file://relative-sourceRoot-source-map.map +// comment +" +`; + +exports[`source-map-loader should reject not valid file: SourceMaps: errors 1`] = `Array []`; + +exports[`source-map-loader should reject not valid file: SourceMaps: warnings 1`] = ` +Array [ + "ModuleWarning: Module Warning (from \`replaced original path\`): +(Emitted value instead of an instance of Error) TypeError [ERR_INVALID_FILE_URL_HOST]: File URL host must be \\"localhost\\" or empty on linux", +] +`; + exports[`source-map-loader should skip invalid base64 SourceMap: css 1`] = ` "without SourceMap // @sourceMappingURL=data:application/source-map;base64,\\"something invalid\\" diff --git a/test/fixtures/file-source-map.js b/test/fixtures/file-source-map.js new file mode 100644 index 0000000..7a66a5e --- /dev/null +++ b/test/fixtures/file-source-map.js @@ -0,0 +1,3 @@ +with SourceMap +//#sourceMappingURL=file://relative-sourceRoot-source-map.map +// comment diff --git a/test/fixtures/http-source-map.js b/test/fixtures/http-source-map.js new file mode 100644 index 0000000..5a476a0 --- /dev/null +++ b/test/fixtures/http-source-map.js @@ -0,0 +1,3 @@ +with SourceMap +//#sourceMappingURL=http://sampledomain.com/external-source-map2.map +// comment diff --git a/test/loader.test.js b/test/loader.test.js index 8500a36..758e8d3 100644 --- a/test/loader.test.js +++ b/test/loader.test.js @@ -80,6 +80,28 @@ describe('source-map-loader', () => { expect(getErrors(stats)).toMatchSnapshot('errors'); }); + it('should reject http SourceMaps', async () => { + const testId = 'http-source-map.js'; + const compiler = getCompiler(testId); + const stats = await compile(compiler); + const codeFromBundle = getCodeFromBundle(stats, compiler); + + expect(codeFromBundle.css).toMatchSnapshot('css'); + expect(getWarnings(stats)).toMatchSnapshot('warnings'); + expect(getErrors(stats)).toMatchSnapshot('errors'); + }); + + it('should reject not valid file: SourceMaps', async () => { + const testId = 'file-source-map.js'; + const compiler = getCompiler(testId); + const stats = await compile(compiler); + const codeFromBundle = getCodeFromBundle(stats, compiler); + + expect(codeFromBundle.css).toMatchSnapshot('css'); + expect(getWarnings(stats)).toMatchSnapshot('warnings'); + expect(getErrors(stats)).toMatchSnapshot('errors'); + }); + it('should use last SourceMap directive', async () => { const testId = 'multi-source-map.js'; const compiler = getCompiler(testId); From 41caa08690101dd1de56b8c6d1adaef0141d867d Mon Sep 17 00:00:00 2001 From: Bernardito Date: Thu, 21 May 2020 23:18:26 +0300 Subject: [PATCH 8/9] fix: absolute path for sources and add sources in dependendency --- test/__snapshots__/loader.test.js.snap | 4 ++-- test/fixtures/file-source-map.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap index ad2bdbd..924f370 100644 --- a/test/__snapshots__/loader.test.js.snap +++ b/test/__snapshots__/loader.test.js.snap @@ -126,7 +126,7 @@ Array [ exports[`source-map-loader should reject not valid file: SourceMaps: css 1`] = ` "with SourceMap -//#sourceMappingURL=file://relative-sourceRoot-source-map.map +//#sourceMappingURL=file:///relative-sourceRoot-source-map.map // comment " `; @@ -136,7 +136,7 @@ exports[`source-map-loader should reject not valid file: SourceMaps: errors 1`] exports[`source-map-loader should reject not valid file: SourceMaps: warnings 1`] = ` Array [ "ModuleWarning: Module Warning (from \`replaced original path\`): -(Emitted value instead of an instance of Error) TypeError [ERR_INVALID_FILE_URL_HOST]: File URL host must be \\"localhost\\" or empty on linux", +(Emitted value instead of an instance of Error) TypeError [ERR_INVALID_FILE_URL_PATH]: File URL path must be absolute", ] `; diff --git a/test/fixtures/file-source-map.js b/test/fixtures/file-source-map.js index 7a66a5e..1b5b428 100644 --- a/test/fixtures/file-source-map.js +++ b/test/fixtures/file-source-map.js @@ -1,3 +1,3 @@ with SourceMap -//#sourceMappingURL=file://relative-sourceRoot-source-map.map +//#sourceMappingURL=file:///relative-sourceRoot-source-map.map // comment From 04e5599d769f22ff6eb91c689e78365b0f0dc90f Mon Sep 17 00:00:00 2001 From: Bernardito Date: Fri, 22 May 2020 00:55:48 +0300 Subject: [PATCH 9/9] fix: absolute path for sources and add sources in dependendency --- test/__snapshots__/loader.test.js.snap | 16 ++-------------- test/fixtures/file-source-map-windows.js | 3 +++ test/fixtures/file-source-map.js | 2 +- test/loader.test.js | 12 +++++++----- 4 files changed, 13 insertions(+), 20 deletions(-) create mode 100644 test/fixtures/file-source-map-windows.js diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap index 924f370..7b4d020 100644 --- a/test/__snapshots__/loader.test.js.snap +++ b/test/__snapshots__/loader.test.js.snap @@ -124,21 +124,9 @@ Array [ ] `; -exports[`source-map-loader should reject not valid file: SourceMaps: css 1`] = ` -"with SourceMap -//#sourceMappingURL=file:///relative-sourceRoot-source-map.map -// comment -" -`; - -exports[`source-map-loader should reject not valid file: SourceMaps: errors 1`] = `Array []`; +exports[`source-map-loader should reject not exist file: SourceMaps: errors 1`] = `Array []`; -exports[`source-map-loader should reject not valid file: SourceMaps: warnings 1`] = ` -Array [ - "ModuleWarning: Module Warning (from \`replaced original path\`): -(Emitted value instead of an instance of Error) TypeError [ERR_INVALID_FILE_URL_PATH]: File URL path must be absolute", -] -`; +exports[`source-map-loader should reject not exist file: SourceMaps: warnings 1`] = `"TypeError [ERR_INVALID_FILE"`; exports[`source-map-loader should skip invalid base64 SourceMap: css 1`] = ` "without SourceMap diff --git a/test/fixtures/file-source-map-windows.js b/test/fixtures/file-source-map-windows.js new file mode 100644 index 0000000..d121cbb --- /dev/null +++ b/test/fixtures/file-source-map-windows.js @@ -0,0 +1,3 @@ +with SourceMap +// #sourceMappingURL=file:///unresolvedPath/map.map +// comment diff --git a/test/fixtures/file-source-map.js b/test/fixtures/file-source-map.js index 1b5b428..d3b51ee 100644 --- a/test/fixtures/file-source-map.js +++ b/test/fixtures/file-source-map.js @@ -1,3 +1,3 @@ with SourceMap -//#sourceMappingURL=file:///relative-sourceRoot-source-map.map +// #sourceMappingURL=file://relative-sourceRoot-source-map.map // comment diff --git a/test/loader.test.js b/test/loader.test.js index 758e8d3..04c57d4 100644 --- a/test/loader.test.js +++ b/test/loader.test.js @@ -11,6 +11,8 @@ import { readAsset, } from './helpers'; +const isWin = process.platform === 'win32'; + describe('source-map-loader', () => { it('should leave normal files untouched', async () => { const testId = 'normal-file.js'; @@ -91,14 +93,14 @@ describe('source-map-loader', () => { expect(getErrors(stats)).toMatchSnapshot('errors'); }); - it('should reject not valid file: SourceMaps', async () => { - const testId = 'file-source-map.js'; + it('should reject not exist file: SourceMaps', async () => { + const testId = isWin ? 'file-source-map-windows.js' : 'file-source-map.js'; const compiler = getCompiler(testId); const stats = await compile(compiler); - const codeFromBundle = getCodeFromBundle(stats, compiler); + const errorString = getWarnings(stats).join(''); + const warning = errorString.match(/TypeError \[ERR_INVALID_FILE/gi); - expect(codeFromBundle.css).toMatchSnapshot('css'); - expect(getWarnings(stats)).toMatchSnapshot('warnings'); + expect(...warning).toMatchSnapshot('warnings'); expect(getErrors(stats)).toMatchSnapshot('errors'); });