From c6e360d22422f924b0c8e3fc97b6e89491032371 Mon Sep 17 00:00:00 2001 From: Permutator Date: Fri, 10 Jun 2016 21:29:10 -0700 Subject: [PATCH 1/8] const map = map? I think not. --- src/utils/transformBundle.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/transformBundle.js b/src/utils/transformBundle.js index f82bfb6..56a4359 100644 --- a/src/utils/transformBundle.js +++ b/src/utils/transformBundle.js @@ -11,7 +11,7 @@ export default function transformBundle ( code, transformers, sourceMapChain ) { }; } - const map = typeof result.map === 'string' ? JSON.parse( result.map ) : map; + const map = typeof result.map === 'string' ? JSON.parse( result.map ) : result.map; sourceMapChain.push( map ); return result.code; From 22f57c4571b938a9b7d89b4b130c2b61229f4e07 Mon Sep 17 00:00:00 2001 From: Permutator Date: Sat, 11 Jun 2016 19:12:54 -0700 Subject: [PATCH 2/8] Made it possible for plugin loaders to provide sourcemaps --- src/Bundle.js | 24 +++++----- src/Module.js | 3 +- src/utils/collapseSourcemaps.js | 72 +++++++++++++++++++++++------- src/utils/transform.js | 4 +- test/sourcemaps/loaders/_config.js | 50 +++++++++++++++++++++ test/sourcemaps/loaders/foo.js | 3 ++ test/sourcemaps/loaders/main.js | 1 + 7 files changed, 128 insertions(+), 29 deletions(-) create mode 100644 test/sourcemaps/loaders/_config.js create mode 100644 test/sourcemaps/loaders/foo.js create mode 100644 test/sourcemaps/loaders/main.js diff --git a/src/Bundle.js b/src/Bundle.js index 3d4414a..d0709fa 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -46,12 +46,11 @@ export default class Bundle { .concat( resolveId ) ); - this.load = first( - this.plugins - .map( plugin => plugin.load ) - .filter( Boolean ) - .concat( load ) - ); + const loaders = this.plugins + .map( plugin => plugin.load ) + .filter( Boolean ); + this.hasLoaders = loaders.length !== 0; + this.load = first( loaders.concat( load ) ); this.transformers = this.plugins .map( plugin => plugin.transform ) @@ -204,9 +203,9 @@ export default class Bundle { return transform( source, id, this.transformers ); }) .then( source => { - const { code, originalCode, ast, sourceMapChain } = source; + const { code, originalCode, originalSourceMap, ast, sourceMapChain } = source; - const module = new Module({ id, code, originalCode, ast, sourceMapChain, bundle: this }); + const module = new Module({ id, code, originalCode, originalSourceMap, ast, sourceMapChain, bundle: this }); this.modules.push( module ); this.moduleById.set( id, module ); @@ -337,10 +336,11 @@ export default class Bundle { let file = options.sourceMapFile || options.dest; if ( file ) file = resolve( typeof process !== 'undefined' ? process.cwd() : '', file ); - map = magicString.generateMap({ file, includeContent: true }); - - if ( this.transformers.length || this.bundleTransformers.length ) { - map = collapseSourcemaps( map, usedModules, bundleSourcemapChain ); + if ( this.hasLoaders || this.transformers.length || this.bundleTransformers.length ) { + map = magicString.generateMap( {} ); + map = collapseSourcemaps( file, map, usedModules, bundleSourcemapChain ); + } else { + map = magicString.generateMap({ file, includeContent: true }); } map.sources = map.sources.map( unixizePath ); diff --git a/src/Module.js b/src/Module.js index 7bc89d3..2cb9a61 100644 --- a/src/Module.js +++ b/src/Module.js @@ -17,9 +17,10 @@ import { emptyBlockStatement } from './ast/create.js'; import extractNames from './ast/extractNames.js'; export default class Module { - constructor ({ id, code, originalCode, ast, sourceMapChain, bundle }) { + constructor ({ id, code, originalCode, originalSourceMap, ast, sourceMapChain, bundle }) { this.code = code; this.originalCode = originalCode; + this.originalSourceMap = originalSourceMap; this.sourceMapChain = sourceMapChain; this.bundle = bundle; diff --git a/src/utils/collapseSourcemaps.js b/src/utils/collapseSourcemaps.js index ff86eaf..b2a6a97 100644 --- a/src/utils/collapseSourcemaps.js +++ b/src/utils/collapseSourcemaps.js @@ -1,13 +1,15 @@ import { encode, decode } from 'sourcemap-codec'; +import { dirname, relative, resolve } from './path.js'; class Source { - constructor ( index ) { + constructor ( filename, content ) { this.isOriginal = true; - this.index = index; + this.filename = filename; + this.content = content; } traceSegment ( line, column, name ) { - return { line, column, name, index: this.index }; + return { line, column, name, source: this }; } } @@ -21,7 +23,7 @@ class Link { } traceMappings () { - let names = []; + let sources = [], sourcesContent = [], names = []; const mappings = this.mappings.map( line => { let tracedLine = []; @@ -31,14 +33,28 @@ class Link { const traced = source.traceSegment( segment[2], segment[3], this.names[ segment[4] ] ); if ( traced ) { - let nameIndex = null; + let sourceIndex = null, nameIndex = null; segment = [ segment[0], - traced.index, + null, traced.line, traced.column ]; + // newer sources are more likely to be used, so search backwards. + sourceIndex = sources.lastIndexOf( traced.source.filename ); + if ( sourceIndex === -1 ) { + sourceIndex = sources.length; + sources.push( traced.source.filename ); + sourcesContent[ sourceIndex ] = traced.source.content; + } else if ( sourcesContent[ sourceIndex ] == null ) { + sourcesContent[ sourceIndex ] = traced.source.content; + } else if ( traced.source.content != null && sourcesContent[ sourceIndex ] !== traced.source.content ) { + throw new Error( `Multiple conflicting contents for sourcemap source ${source.filename}` ); + } + + segment[1] = sourceIndex; + if ( traced.name ) { nameIndex = names.indexOf( traced.name ); if ( nameIndex === -1 ) { @@ -56,7 +72,7 @@ class Link { return tracedLine; }); - return { names, mappings }; + return { sources, sourcesContent, names, mappings }; } traceSegment ( line, column, name ) { @@ -81,29 +97,55 @@ class Link { } } -export default function collapseSourcemaps ( map, modules, bundleSourcemapChain ) { - const sources = modules.map( ( module, i ) => { - let source = new Source( i ); +export default function collapseSourcemaps ( file, map, modules, bundleSourcemapChain ) { + const moduleSources = modules.map( module => { + let sourceMapChain = module.sourceMapChain; + + let source; + if ( module.originalSourceMap == null ) { + source = new Source( module.id, module.originalCode ); + } else { + const sources = module.originalSourceMap.sources; + if ( sources == null || ( sources.length <= 1 && sources[0] == null ) ) { + source = new Source( module.id, module.originalCode ); + sourceMapChain = [ module.originalSourceMap ].concat( sourceMapChain ); + } else { + // TODO indiscriminately treating IDs and sources as normal paths is probably bad. + const sourcesContent = module.originalSourceMap.sourcesContent || []; + const directory = dirname( module.id ) || '.'; + const sourceRoot = module.originalSourceMap.sourceRoot || '.'; + const baseSources = sources.map( (source, i) => { + return new Source( resolve( directory, sourceRoot, source ), sourcesContent[i] ); + }); + source = new Link( module.originalSourceMap, baseSources ); + } + } - module.sourceMapChain.forEach( map => { + sourceMapChain.forEach( map => { source = new Link( map, [ source ]); }); return source; }); - let source = new Link( map, sources ); + let source = new Link( map, moduleSources ); bundleSourcemapChain.forEach( map => { source = new Link( map, [ source ] ); }); - const { names, mappings } = source.traceMappings(); + let { sources, sourcesContent, names, mappings } = source.traceMappings(); + + if ( file ) { + const directory = dirname( file ); + sources = sources.map( source => relative( directory, source ) ); + } // we re-use the `map` object because it has convenient toString/toURL methods - map.sourcesContent = modules.map( module => module.originalCode ); - map.mappings = encode( mappings ); + map.sources = sources; + map.sourcesContent = sourcesContent; map.names = names; + map.mappings = encode( mappings ); return map; } diff --git a/src/utils/transform.js b/src/utils/transform.js index 62b47e2..285eea0 100644 --- a/src/utils/transform.js +++ b/src/utils/transform.js @@ -1,6 +1,8 @@ export default function transform ( source, id, transformers ) { let sourceMapChain = []; + const originalSourceMap = typeof source.map === 'string' ? JSON.parse( source.map ) : source.map; + let originalCode = source.code; let ast = source.ast; @@ -30,7 +32,7 @@ export default function transform ( source, id, transformers ) { }, Promise.resolve( source.code ) ) - .then( code => ({ code, originalCode, ast, sourceMapChain }) ) + .then( code => ({ code, originalCode, originalSourceMap, ast, sourceMapChain }) ) .catch( err => { err.id = id; err.message = `Error loading ${id}: ${err.message}`; diff --git a/test/sourcemaps/loaders/_config.js b/test/sourcemaps/loaders/_config.js new file mode 100644 index 0000000..ea8e90e --- /dev/null +++ b/test/sourcemaps/loaders/_config.js @@ -0,0 +1,50 @@ +var babel = require( 'babel-core' ); +var fs = require( 'fs' ); +var assert = require( 'assert' ); +var getLocation = require( '../../utils/getLocation' ); +var SourceMapConsumer = require( 'source-map' ).SourceMapConsumer; + +module.exports = { + description: 'preserves sourcemap chains when transforming', + options: { + plugins: [ + { + load: function ( id ) { + if ( id.endsWith( 'main.js' ) ) { + id = id.replace( /main.js$/, 'foo.js' ); + } else { + id = id.replace( /foo.js$/, 'main.js' ); + } + + var out = babel.transformFileSync( id, { + blacklist: [ 'es6.modules' ], + sourceMap: true + }); + + const sourceRoot = out.map.sources[0].slice( 0, out.map.sources[0].lastIndexOf( '/' ) ); + out.map.sources = out.map.sources.map( source => '../' + source.slice( sourceRoot.length + 1 ) ); + out.map.sourceRoot = 'fake'; + + return { code: out.code, map: out.map }; + } + } + ] + }, + test: function ( code, map ) { + var smc = new SourceMapConsumer( map ); + + var generatedLoc = getLocation( code, code.indexOf( '42' ) ); + var originalLoc = smc.originalPositionFor( generatedLoc ); + + assert.equal( originalLoc.source, '../main.js' ); + assert.equal( originalLoc.line, 1 ); + assert.equal( originalLoc.column, 25 ); + + generatedLoc = getLocation( code, code.indexOf( 'log' ) ); + originalLoc = smc.originalPositionFor( generatedLoc ); + + assert.equal( originalLoc.source, '../foo.js' ); + assert.equal( originalLoc.line, 3 ); + assert.equal( originalLoc.column, 8 ); + } +}; diff --git a/test/sourcemaps/loaders/foo.js b/test/sourcemaps/loaders/foo.js new file mode 100644 index 0000000..c0b4c17 --- /dev/null +++ b/test/sourcemaps/loaders/foo.js @@ -0,0 +1,3 @@ +import { foo } from './foo'; + +console.log( `the answer is ${foo()}` ); diff --git a/test/sourcemaps/loaders/main.js b/test/sourcemaps/loaders/main.js new file mode 100644 index 0000000..0cf01be --- /dev/null +++ b/test/sourcemaps/loaders/main.js @@ -0,0 +1 @@ +export const foo = () => 42; From 3f39f2aec4ea8c83abdd1942a768f8be8fb4fa40 Mon Sep 17 00:00:00 2001 From: Permutator Date: Sat, 11 Jun 2016 19:19:32 -0700 Subject: [PATCH 3/8] Simplify sourcemaps loaders test --- test/sourcemaps/loaders/_config.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/sourcemaps/loaders/_config.js b/test/sourcemaps/loaders/_config.js index ea8e90e..cf13906 100644 --- a/test/sourcemaps/loaders/_config.js +++ b/test/sourcemaps/loaders/_config.js @@ -21,8 +21,8 @@ module.exports = { sourceMap: true }); - const sourceRoot = out.map.sources[0].slice( 0, out.map.sources[0].lastIndexOf( '/' ) ); - out.map.sources = out.map.sources.map( source => '../' + source.slice( sourceRoot.length + 1 ) ); + const slash = out.map.sources[0].lastIndexOf( '/' ) + 1; + out.map.sources = out.map.sources.map( source => '../' + source.slice( slash ) ); out.map.sourceRoot = 'fake'; return { code: out.code, map: out.map }; From 4ebaeecb1514ed9f6f2926c0cf3c4ffc63e638eb Mon Sep 17 00:00:00 2001 From: Permutator Date: Sat, 11 Jun 2016 19:24:37 -0700 Subject: [PATCH 4/8] Reduce blobbiness of collapseSourcemaps --- src/utils/collapseSourcemaps.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/utils/collapseSourcemaps.js b/src/utils/collapseSourcemaps.js index b2a6a97..763998f 100644 --- a/src/utils/collapseSourcemaps.js +++ b/src/utils/collapseSourcemaps.js @@ -106,17 +106,21 @@ export default function collapseSourcemaps ( file, map, modules, bundleSourcemap source = new Source( module.id, module.originalCode ); } else { const sources = module.originalSourceMap.sources; + if ( sources == null || ( sources.length <= 1 && sources[0] == null ) ) { source = new Source( module.id, module.originalCode ); sourceMapChain = [ module.originalSourceMap ].concat( sourceMapChain ); } else { - // TODO indiscriminately treating IDs and sources as normal paths is probably bad. const sourcesContent = module.originalSourceMap.sourcesContent || []; + + // TODO indiscriminately treating IDs and sources as normal paths is probably bad. const directory = dirname( module.id ) || '.'; const sourceRoot = module.originalSourceMap.sourceRoot || '.'; + const baseSources = sources.map( (source, i) => { return new Source( resolve( directory, sourceRoot, source ), sourcesContent[i] ); }); + source = new Link( module.originalSourceMap, baseSources ); } } From 7535c518ff16c1c93f64ef68e9149058186e831e Mon Sep 17 00:00:00 2001 From: Permutator Date: Sat, 11 Jun 2016 19:55:49 -0700 Subject: [PATCH 5/8] Fixed loading of source content from loader-provided sourcemaps --- src/utils/collapseSourcemaps.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/utils/collapseSourcemaps.js b/src/utils/collapseSourcemaps.js index 763998f..68a97e0 100644 --- a/src/utils/collapseSourcemaps.js +++ b/src/utils/collapseSourcemaps.js @@ -100,19 +100,18 @@ class Link { export default function collapseSourcemaps ( file, map, modules, bundleSourcemapChain ) { const moduleSources = modules.map( module => { let sourceMapChain = module.sourceMapChain; - + let source; if ( module.originalSourceMap == null ) { source = new Source( module.id, module.originalCode ); } else { const sources = module.originalSourceMap.sources; + const sourcesContent = module.originalSourceMap.sourcesContent || []; if ( sources == null || ( sources.length <= 1 && sources[0] == null ) ) { - source = new Source( module.id, module.originalCode ); + source = new Source( module.id, sourcesContent[0] ); sourceMapChain = [ module.originalSourceMap ].concat( sourceMapChain ); } else { - const sourcesContent = module.originalSourceMap.sourcesContent || []; - // TODO indiscriminately treating IDs and sources as normal paths is probably bad. const directory = dirname( module.id ) || '.'; const sourceRoot = module.originalSourceMap.sourceRoot || '.'; From 2a7a1a70904d10c8ada50df8177f823cfb5fb295 Mon Sep 17 00:00:00 2001 From: Permutator Date: Sat, 11 Jun 2016 19:56:16 -0700 Subject: [PATCH 6/8] Updated test for loader sourcemaps --- test/sourcemaps/loaders/_config.js | 42 ++++++++++++++++++++---------- test/sourcemaps/loaders/bar.js | 1 + test/sourcemaps/loaders/foo.js | 4 +-- test/sourcemaps/loaders/main.js | 5 +++- 4 files changed, 34 insertions(+), 18 deletions(-) create mode 100644 test/sourcemaps/loaders/bar.js diff --git a/test/sourcemaps/loaders/_config.js b/test/sourcemaps/loaders/_config.js index cf13906..d278904 100644 --- a/test/sourcemaps/loaders/_config.js +++ b/test/sourcemaps/loaders/_config.js @@ -10,20 +10,26 @@ module.exports = { plugins: [ { load: function ( id ) { - if ( id.endsWith( 'main.js' ) ) { - id = id.replace( /main.js$/, 'foo.js' ); - } else { - id = id.replace( /foo.js$/, 'main.js' ); + if ( id.endsWith( 'foo.js' ) ) { + id = id.replace( /foo.js$/, 'bar.js' ); + } else if ( id.endsWith( 'bar.js' ) ) { + id = id.replace( /bar.js$/, 'foo.js' ); } var out = babel.transformFileSync( id, { blacklist: [ 'es6.modules' ], - sourceMap: true + sourceMap: true, + comments: false // misalign the columns }); - const slash = out.map.sources[0].lastIndexOf( '/' ) + 1; - out.map.sources = out.map.sources.map( source => '../' + source.slice( slash ) ); - out.map.sourceRoot = 'fake'; + if ( id.endsWith( 'main.js' ) ) { + delete out.map.sources; + //throw new Error(JSON.stringify(out.code)); + } else { + const slash = out.map.sources[0].lastIndexOf( '/' ) + 1; + out.map.sources = out.map.sources.map( source => '../' + source.slice( slash ) ); + out.map.sourceRoot = 'fake'; + } return { code: out.code, map: out.map }; } @@ -33,18 +39,26 @@ module.exports = { test: function ( code, map ) { var smc = new SourceMapConsumer( map ); - var generatedLoc = getLocation( code, code.indexOf( '42' ) ); + var generatedLoc = getLocation( code, code.indexOf( '22' ) ); var originalLoc = smc.originalPositionFor( generatedLoc ); - assert.equal( originalLoc.source, '../main.js' ); + assert.equal( originalLoc.source, '../foo.js' ); assert.equal( originalLoc.line, 1 ); - assert.equal( originalLoc.column, 25 ); + assert.equal( originalLoc.column, 32 ); + + var generatedLoc = getLocation( code, code.indexOf( '20' ) ); + var originalLoc = smc.originalPositionFor( generatedLoc ); + + assert.equal( originalLoc.source, '../bar.js' ); + assert.equal( originalLoc.line, 1 ); + assert.equal( originalLoc.column, 37 ); generatedLoc = getLocation( code, code.indexOf( 'log' ) ); originalLoc = smc.originalPositionFor( generatedLoc ); - assert.equal( originalLoc.source, '../foo.js' ); - assert.equal( originalLoc.line, 3 ); - assert.equal( originalLoc.column, 8 ); + assert.equal( originalLoc.source, '../main.js' ); + assert.ok( /columns/.test( smc.sourceContentFor( '../main.js' ) ) ); + assert.equal( originalLoc.line, 4 ); + assert.equal( originalLoc.column, 19 ); } }; diff --git a/test/sourcemaps/loaders/bar.js b/test/sourcemaps/loaders/bar.js new file mode 100644 index 0000000..6c3e77f --- /dev/null +++ b/test/sourcemaps/loaders/bar.js @@ -0,0 +1 @@ +/*misalign*/export const foo = () => 20; diff --git a/test/sourcemaps/loaders/foo.js b/test/sourcemaps/loaders/foo.js index c0b4c17..9ce4283 100644 --- a/test/sourcemaps/loaders/foo.js +++ b/test/sourcemaps/loaders/foo.js @@ -1,3 +1 @@ -import { foo } from './foo'; - -console.log( `the answer is ${foo()}` ); +/*the*/export const bar = () => 22; diff --git a/test/sourcemaps/loaders/main.js b/test/sourcemaps/loaders/main.js index 0cf01be..df7e27a 100644 --- a/test/sourcemaps/loaders/main.js +++ b/test/sourcemaps/loaders/main.js @@ -1 +1,4 @@ -export const foo = () => 42; +import { foo } from './foo'; +import { bar } from './bar'; + +/*columns*/console.log( `the answer is ${foo() + bar()}` ); From e256ddb727d4dc4ebb134781e33880cdb62805c0 Mon Sep 17 00:00:00 2001 From: Permutator Date: Sat, 11 Jun 2016 20:16:15 -0700 Subject: [PATCH 7/8] Removed debugging line in sourcemaps loaders test --- test/sourcemaps/loaders/_config.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/sourcemaps/loaders/_config.js b/test/sourcemaps/loaders/_config.js index d278904..2127639 100644 --- a/test/sourcemaps/loaders/_config.js +++ b/test/sourcemaps/loaders/_config.js @@ -24,7 +24,6 @@ module.exports = { if ( id.endsWith( 'main.js' ) ) { delete out.map.sources; - //throw new Error(JSON.stringify(out.code)); } else { const slash = out.map.sources[0].lastIndexOf( '/' ) + 1; out.map.sources = out.map.sources.map( source => '../' + source.slice( slash ) ); From 3a2f9b6e15668837ec6e8306075b6b24da82a53a Mon Sep 17 00:00:00 2001 From: Permutator Date: Sat, 11 Jun 2016 21:42:15 -0700 Subject: [PATCH 8/8] Don't use endsWith. Will this fix the 0.12 build? --- test/sourcemaps/loaders/_config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/sourcemaps/loaders/_config.js b/test/sourcemaps/loaders/_config.js index 2127639..8665133 100644 --- a/test/sourcemaps/loaders/_config.js +++ b/test/sourcemaps/loaders/_config.js @@ -10,9 +10,9 @@ module.exports = { plugins: [ { load: function ( id ) { - if ( id.endsWith( 'foo.js' ) ) { + if ( /foo.js$/.test( id ) ) { id = id.replace( /foo.js$/, 'bar.js' ); - } else if ( id.endsWith( 'bar.js' ) ) { + } else if ( /bar.js$/.test( id ) ) { id = id.replace( /bar.js$/, 'foo.js' ); } @@ -22,7 +22,7 @@ module.exports = { comments: false // misalign the columns }); - if ( id.endsWith( 'main.js' ) ) { + if ( /main.js$/.test( id ) ) { delete out.map.sources; } else { const slash = out.map.sources[0].lastIndexOf( '/' ) + 1;