From 9435773b1ccf84f048ae07ef29ec520a235b1c67 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 25 Jul 2015 12:56:26 -0400 Subject: [PATCH 01/11] start moving render logic into modules --- src/Bundle.js | 157 ++++++++++++++++++++++++++++++----------- src/Module.js | 177 ++++++++++++++++++++++++++++++++++++++++++----- src/Statement.js | 18 ++--- src/rollup.js | 4 +- 4 files changed, 283 insertions(+), 73 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index 4ed55a0..452b3d0 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -13,15 +13,6 @@ import getExportMode from './utils/getExportMode'; import getIndentString from './utils/getIndentString'; import { unixizePath } from './utils/normalizePlatform.js'; -function isEmptyExportedVarDeclaration ( node, module, allBundleExports, es6 ) { - if ( node.type !== 'VariableDeclaration' || node.declarations[0].init ) return false; - - const name = node.declarations[0].id.name; - const canonicalName = module.getCanonicalName( name, es6 ); - - return canonicalName in allBundleExports; -} - export default class Bundle { constructor ( options ) { this.entry = options.entry; @@ -90,7 +81,7 @@ export default class Bundle { return this.markAllModifierStatements(); }) .then( () => { - this.statements = this.sort(); + this.orderedModules = this.sort(); }); } @@ -115,34 +106,35 @@ export default class Bundle { }); // Discover conflicts (i.e. two statements in separate modules both define `foo`) - this.statements.forEach( statement => { - const module = statement.module; - const names = keys( statement.defines ); + this.orderedModules.forEach( module => { + module.statements.forEach( statement => { + const names = keys( statement.defines ); - // with default exports that are expressions (`export default 42`), - // we need to ensure that the name chosen for the expression does - // not conflict - if ( statement.node.type === 'ExportDefaultDeclaration' ) { - const name = module.getCanonicalName( 'default', es6 ); + // with default exports that are expressions (`export default 42`), + // we need to ensure that the name chosen for the expression does + // not conflict + if ( statement.node.type === 'ExportDefaultDeclaration' ) { + const name = module.getCanonicalName( 'default', es6 ); - const isProxy = statement.node.declaration && statement.node.declaration.type === 'Identifier'; - const shouldDeconflict = !isProxy || ( module.getCanonicalName( statement.node.declaration.name, es6 ) !== name ); + const isProxy = statement.node.declaration && statement.node.declaration.type === 'Identifier'; + const shouldDeconflict = !isProxy || ( module.getCanonicalName( statement.node.declaration.name, es6 ) !== name ); - if ( shouldDeconflict && !~names.indexOf( name ) ) { - names.push( name ); + if ( shouldDeconflict && !~names.indexOf( name ) ) { + names.push( name ); + } } - } - names.forEach( name => { - if ( definers[ name ] ) { - conflicts[ name ] = true; - } else { - definers[ name ] = []; - } + names.forEach( name => { + if ( definers[ name ] ) { + conflicts[ name ] = true; + } else { + definers[ name ] = []; + } - // TODO in good js, there shouldn't be duplicate definitions - // per module... but some people write bad js - definers[ name ].push( module ); + // TODO in good js, there shouldn't be duplicate definitions + // per module... but some people write bad js + definers[ name ].push( module ); + }); }); }); @@ -459,6 +451,97 @@ export default class Bundle { }); } + render ( options = {} ) { + const format = options.format || 'es6'; + this.deconflict( format === 'es6' ); + + // If we have named exports from the bundle, and those exports + // are assigned to *within* the bundle, we may need to rewrite e.g. + // + // export let count = 0; + // export function incr () { count++ } + // + // might become... + // + // exports.count = 0; + // function incr () { + // exports.count += 1; + // } + // exports.incr = incr; + // + // This doesn't apply if the bundle is exported as ES6! + let allBundleExports = blank(); + + if ( format !== 'es6' ) { + keys( this.entryModule.exports ).forEach( key => { + const exportDeclaration = this.entryModule.exports[ key ]; + + const originalDeclaration = this.entryModule.findDeclaration( exportDeclaration.localName ); + + if ( originalDeclaration && originalDeclaration.type === 'VariableDeclaration' ) { + const canonicalName = this.entryModule.getCanonicalName( exportDeclaration.localName, false ); + + allBundleExports[ canonicalName ] = `exports.${key}`; + this.varExports[ key ] = true; + } + }); + } + + // since we're rewriting variable exports, we want to + // ensure we don't try and export them again at the bottom + this.toExport = keys( this.entryModule.exports ) + .filter( key => !this.varExports[ key ] ); + + + let magicString = new MagicString.Bundle({ separator: '\n\n' }); + + this.orderedModules.forEach( module => { + magicString.addSource( module.render( allBundleExports, format ) ); + }); + + // prepend bundle with internal namespaces + const indentString = magicString.getIndentString(); + const namespaceBlock = this.internalNamespaceModules.map( module => { + const exportKeys = keys( module.exports ); + + return `var ${module.getCanonicalName('*', format === 'es6')} = {\n` + + exportKeys.map( key => `${indentString}get ${key} () { return ${module.getCanonicalName(key, format === 'es6')}; }` ).join( ',\n' ) + + `\n};\n\n`; + }).join( '' ); + + magicString.prepend( namespaceBlock ); + + const finalise = finalisers[ format ]; + + if ( !finalise ) { + throw new Error( `You must specify an output type - valid options are ${keys( finalisers ).join( ', ' )}` ); + } + + magicString = finalise( this, magicString.trim(), { + // Determine export mode - 'default', 'named', 'none' + exportMode: getExportMode( this, options.exports ), + + // Determine indentation + indentString: getIndentString( magicString, options ) + }, options ); + + const code = magicString.toString(); + let map = null; + + if ( options.sourceMap ) { + const file = options.sourceMapFile || options.dest; + map = magicString.generateMap({ + includeContent: true, + file + // TODO + }); + + map.sources = map.sources.map( unixizePath ); + } + + return { code, map }; + } + sort () { let seen = {}; let ordered = []; @@ -540,14 +623,6 @@ export default class Bundle { }); } - let statements = []; - - ordered.forEach( module => { - module.statements.forEach( statement => { - if ( statement.isIncluded ) statements.push( statement ); - }); - }); - - return statements; + return ordered; } } diff --git a/src/Module.js b/src/Module.js index a26e340..907e540 100644 --- a/src/Module.js +++ b/src/Module.js @@ -21,6 +21,15 @@ function deconflict ( name, names ) { return name; } +function isEmptyExportedVarDeclaration ( node, module, allBundleExports, es6 ) { + if ( node.type !== 'VariableDeclaration' || node.declarations[0].init ) return false; + + const name = node.declarations[0].id.name; + const canonicalName = module.getCanonicalName( name, es6 ); + + return canonicalName in allBundleExports; +} + export default class Module { constructor ({ id, source, bundle }) { this.source = source; @@ -531,36 +540,41 @@ export default class Module { let statements = []; - ast.body.map( node => { + ast.body.forEach( node => { // special case - top-level var declarations with multiple declarators // should be split up. Otherwise, we may end up including code we // don't need, just because an unwanted declarator is included if ( node.type === 'VariableDeclaration' && node.declarations.length > 1 ) { - node.declarations.forEach( declarator => { - const magicString = this.magicString.snip( declarator.start, declarator.end ).trim(); - magicString.prepend( `${node.kind} ` ).append( ';' ); - - const syntheticNode = { - type: 'VariableDeclaration', - kind: node.kind, - start: node.start, - end: node.end, - declarations: [ declarator ] - }; - - const statement = new Statement( syntheticNode, magicString, this, statements.length ); - statements.push( statement ); - }); + throw new Error( 'TODO' ); + // node.declarations.forEach( declarator => { + // const magicString = this.magicString.snip( declarator.start, declarator.end ).trim(); + // magicString.prepend( `${node.kind} ` ).append( ';' ); + // + // const syntheticNode = { + // type: 'VariableDeclaration', + // kind: node.kind, + // start: node.start, + // end: node.end, + // declarations: [ declarator ] + // }; + // + // const statement = new Statement( syntheticNode, magicString, this, statements.length ); + // statements.push( statement ); + // }); } else { - const magicString = this.magicString.snip( node.start, node.end ).trim(); - const statement = new Statement( node, magicString, this, statements.length ); + const statement = new Statement( node, this, node.start, node.end ); // TODO should be comment start, comment end statements.push( statement ); } }); + statements.forEach( ( statement, i ) => { + const nextStatement = statements[ i + 1 ]; + statement.next = nextStatement ? nextStatement.start : statement.end; + }); + return statements; } @@ -569,6 +583,133 @@ export default class Module { this.canonicalNames[ name ] = this.canonicalNames[ name + '-es6' ] = replacement; } + render ( allBundleExports, format ) { + let magicString = this.magicString.clone(); + + let previousIndex = -1; + let previousMargin = 0; + + this.statements.forEach( statement => { + if ( !statement.isIncluded ) { + magicString.remove( statement.start, statement.next ); + return; + } + + // skip `export { foo, bar, baz }` + if ( statement.node.type === 'ExportNamedDeclaration' ) { + // skip `export { foo, bar, baz }` + if ( statement.node.specifiers.length ) { + magicString.remove( statement.start, statement.next ); + return; + }; + + // skip `export var foo;` if foo is exported + if ( isEmptyExportedVarDeclaration( statement.node.declaration, statement.module, allBundleExports, format === 'es6' ) ) { + magicString.remove( statement.start, statement.next ); + return; + } + } + + // skip empty var declarations for exported bindings + // (otherwise we're left with `exports.foo;`, which is useless) + if ( isEmptyExportedVarDeclaration( statement.node, statement.module, allBundleExports, format === 'es6' ) ) { + magicString.remove( statement.start, statement.next ); + return; + } + + let replacements = blank(); + let bundleExports = blank(); + + keys( statement.dependsOn ) + .concat( keys( statement.defines ) ) + .forEach( name => { + const canonicalName = statement.module.getCanonicalName( name, format === 'es6' ); + + if ( allBundleExports[ canonicalName ] ) { + bundleExports[ name ] = replacements[ name ] = allBundleExports[ canonicalName ]; + } else if ( name !== canonicalName ) { + replacements[ name ] = canonicalName; + } + }); + + statement.replaceIdentifiers( magicString, replacements, bundleExports ); + + // modify exports as necessary + if ( statement.isExportDeclaration ) { + // remove `export` from `export var foo = 42` + if ( statement.node.type === 'ExportNamedDeclaration' && statement.node.declaration.type === 'VariableDeclaration' ) { + magicString.remove( statement.node.start, statement.node.declaration.start ); + } + + // remove `export` from `export class Foo {...}` or `export default Foo` + // TODO default exports need different treatment + else if ( statement.node.declaration.id ) { + magicString.remove( statement.node.start, statement.node.declaration.start ); + } + + else if ( statement.node.type === 'ExportDefaultDeclaration' ) { + const module = statement.module; + const canonicalName = module.getCanonicalName( 'default', format === 'es6' ); + + if ( statement.node.declaration.type === 'Identifier' && canonicalName === module.getCanonicalName( statement.node.declaration.name, format === 'es6' ) ) { + magicString.remove( statement.start, statement.next ); + return; + } + + // anonymous functions should be converted into declarations + if ( statement.node.declaration.type === 'FunctionExpression' ) { + magicString.overwrite( statement.node.start, statement.node.declaration.start + 8, `function ${canonicalName}` ); + } else { + magicString.overwrite( statement.node.start, statement.node.declaration.start, `var ${canonicalName} = ` ); + } + } + + else { + throw new Error( 'Unhandled export' ); + } + } + + // // ensure there is always a newline between statements, and add + // // additional newlines as necessary to reflect original source + // const minSeparation = ( statement.index !== previousIndex + 1 ) ? 3 : 2; + // const margin = Math.max( minSeparation, statement.margin[0], previousMargin ); + // let newLines = new Array( margin ).join( '\n' ); + // + // // add leading comments + // if ( statement.leadingComments.length ) { + // const commentBlock = newLines + statement.leadingComments.map( ({ separator, comment }) => { + // return separator + ( comment.block ? + // `/*${comment.text}*/` : + // `//${comment.text}` ); + // }).join( '' ); + // + // magicString.addSource( new MagicString( commentBlock ) ); + // newLines = new Array( statement.margin[0] ).join( '\n' ); // TODO handle gaps between comment block and statement + // } + // + // // add the statement itself + // magicString.addSource({ + // content: source, + // separator: newLines + // }); + // + // // add trailing comments + // const comment = statement.trailingComment; + // if ( comment ) { + // const commentBlock = comment.block ? + // ` /*${comment.text}*/` : + // ` //${comment.text}`; + // + // magicString.append( commentBlock ); + // } + // + // previousMargin = statement.margin[1]; + // previousIndex = statement.index; + }); + + return magicString; + } + suggestName ( defaultOrBatch, suggestion ) { // deconflict anonymous default exports with this module's definitions const shouldDeconflict = this.exports.default && this.exports.default.isAnonymous; diff --git a/src/Statement.js b/src/Statement.js index 7cfbebf..9012d46 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -9,12 +9,12 @@ function isIife ( node, parent ) { } export default class Statement { - constructor ( node, magicString, module, index ) { + constructor ( node, module, start, end ) { this.node = node; this.module = module; - this.magicString = magicString; - this.index = index; - this.id = module.id + '#' + index; + this.start = start; + this.end = end; + this.next = null; // filled in later this.scope = new Scope(); this.defines = blank(); @@ -37,17 +37,12 @@ export default class Statement { analyse () { if ( this.isImportDeclaration ) return; // nothing to analyse - const statement = this; // TODO use arrow functions instead - const magicString = this.magicString; - let scope = this.scope; walk( this.node, { enter ( node, parent ) { let newScope; - magicString.addSourcemapLocation( node.start ); - switch ( node.type ) { case 'FunctionExpression': case 'FunctionDeclaration': @@ -146,7 +141,7 @@ export default class Statement { } keys( scope.declarations ).forEach( name => { - statement.defines[ name ] = true; + this.defines[ name ] = true; }); } @@ -247,8 +242,7 @@ export default class Statement { }); } - replaceIdentifiers ( names, bundleExports ) { - const magicString = this.magicString.clone(); + replaceIdentifiers ( magicString, names, bundleExports ) { const replacementStack = [ names ]; const nameList = keys( names ); diff --git a/src/rollup.js b/src/rollup.js index 0b8a652..846c8f2 100644 --- a/src/rollup.js +++ b/src/rollup.js @@ -14,14 +14,14 @@ export function rollup ( options ) { return bundle.build().then( () => { return { - generate: options => bundle.generate( options ), + generate: options => bundle.render( options ), write: options => { if ( !options || !options.dest ) { throw new Error( 'You must supply options.dest to bundle.write' ); } const dest = options.dest; - let { code, map } = bundle.generate( options ); + let { code, map } = bundle.render( options ); let promises = []; From 6d2e108c018450eaa476ed30baa7d24cc5fd56af Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 25 Jul 2015 13:07:27 -0400 Subject: [PATCH 02/11] reimplement multiple var splitting --- src/Module.js | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/Module.js b/src/Module.js index 907e540..b40008f 100644 --- a/src/Module.js +++ b/src/Module.js @@ -545,22 +545,26 @@ export default class Module { // should be split up. Otherwise, we may end up including code we // don't need, just because an unwanted declarator is included if ( node.type === 'VariableDeclaration' && node.declarations.length > 1 ) { - throw new Error( 'TODO' ); - // node.declarations.forEach( declarator => { - // const magicString = this.magicString.snip( declarator.start, declarator.end ).trim(); - // magicString.prepend( `${node.kind} ` ).append( ';' ); - // - // const syntheticNode = { - // type: 'VariableDeclaration', - // kind: node.kind, - // start: node.start, - // end: node.end, - // declarations: [ declarator ] - // }; - // - // const statement = new Statement( syntheticNode, magicString, this, statements.length ); - // statements.push( statement ); - // }); + + node.declarations.forEach( ( declarator, i ) => { + //const magicString = this.magicString.snip( declarator.start, declarator.end ).trim(); + const nextDeclarator = node.declarations[ i + 1 ]; + + if ( nextDeclarator ) { + this.magicString.overwrite( declarator.end, nextDeclarator.start, `;\n${node.kind} ` ); // TODO indentation + } + + const syntheticNode = { + type: 'VariableDeclaration', + kind: node.kind, + start: node.start, + end: node.end, + declarations: [ declarator ] + }; + + const statement = new Statement( syntheticNode, this, node.start, node.end ); // TODO this is almost certainly wrong... + statements.push( statement ); + }); } else { @@ -707,7 +711,7 @@ export default class Module { // previousIndex = statement.index; }); - return magicString; + return magicString.trim(); } suggestName ( defaultOrBatch, suggestion ) { From 8f3f437e1cdd75de8ecf48313095db9a6104e0ef Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 25 Jul 2015 14:14:47 -0400 Subject: [PATCH 03/11] reattach comments, handle var splitting better --- src/Bundle.js | 5 ++- src/Module.js | 31 +++++++++++++++---- .../form/exported-empty-vars/_expected/es6.js | 3 +- .../self-contained-bundle/_expected/amd.js | 1 - .../self-contained-bundle/_expected/cjs.js | 1 - .../self-contained-bundle/_expected/es6.js | 1 - .../self-contained-bundle/_expected/iife.js | 1 - .../self-contained-bundle/_expected/umd.js | 1 - 8 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index 452b3d0..85592f8 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -496,7 +496,10 @@ export default class Bundle { let magicString = new MagicString.Bundle({ separator: '\n\n' }); this.orderedModules.forEach( module => { - magicString.addSource( module.render( allBundleExports, format ) ); + const source = module.render( allBundleExports, format ); + if ( source.toString().length ) { + magicString.addSource( source ); + } }); // prepend bundle with internal namespaces diff --git a/src/Module.js b/src/Module.js index b40008f..831a748 100644 --- a/src/Module.js +++ b/src/Module.js @@ -539,6 +539,8 @@ export default class Module { }); let statements = []; + let lastChar = 0; + let commentIndex = 0; ast.body.forEach( node => { // special case - top-level var declarations with multiple declarators @@ -550,9 +552,9 @@ export default class Module { //const magicString = this.magicString.snip( declarator.start, declarator.end ).trim(); const nextDeclarator = node.declarations[ i + 1 ]; - if ( nextDeclarator ) { - this.magicString.overwrite( declarator.end, nextDeclarator.start, `;\n${node.kind} ` ); // TODO indentation - } + // if ( nextDeclarator ) { + // this.magicString.overwrite( declarator.end, nextDeclarator.start, `;\n${node.kind} ` ); // TODO indentation + // } const syntheticNode = { type: 'VariableDeclaration', @@ -562,15 +564,32 @@ export default class Module { declarations: [ declarator ] }; - const statement = new Statement( syntheticNode, this, node.start, node.end ); // TODO this is almost certainly wrong... + const start = i === 0 ? node.start : declarator.start; + const end = declarator.end; + + const statement = new Statement( syntheticNode, this, start, end ); // TODO this is almost certainly wrong... statements.push( statement ); }); + + lastChar = node.end; // TODO account for trailing line comment } else { - const statement = new Statement( node, this, node.start, node.end ); // TODO should be comment start, comment end - + let comment; + do { + comment = this.comments[ commentIndex ]; + if ( !comment ) break; + if ( comment.start > node.start ) break; + commentIndex += 1; + } while ( comment.end < lastChar ); + + const start = comment ? Math.min( comment.start, node.start ) : node.start; + const end = node.end; // TODO account for trailing line comment + + const statement = new Statement( node, this, start, end ); statements.push( statement ); + + lastChar = end; } }); diff --git a/test/form/exported-empty-vars/_expected/es6.js b/test/form/exported-empty-vars/_expected/es6.js index 8151c68..8c8cdc3 100644 --- a/test/form/exported-empty-vars/_expected/es6.js +++ b/test/form/exported-empty-vars/_expected/es6.js @@ -1,8 +1,7 @@ var foo; foo = 42; -var bar; -var baz; +var bar, baz; bar = 43; baz = 44; diff --git a/test/form/self-contained-bundle/_expected/amd.js b/test/form/self-contained-bundle/_expected/amd.js index 5638a53..ed04c7c 100644 --- a/test/form/self-contained-bundle/_expected/amd.js +++ b/test/form/self-contained-bundle/_expected/amd.js @@ -8,7 +8,6 @@ define(function () { 'use strict'; return 42; } - // comment before 1 console.log( 1 ); diff --git a/test/form/self-contained-bundle/_expected/cjs.js b/test/form/self-contained-bundle/_expected/cjs.js index 8b0b5bd..8a71fe7 100644 --- a/test/form/self-contained-bundle/_expected/cjs.js +++ b/test/form/self-contained-bundle/_expected/cjs.js @@ -8,7 +8,6 @@ function bar () { return 42; } - // comment before 1 console.log( 1 ); diff --git a/test/form/self-contained-bundle/_expected/es6.js b/test/form/self-contained-bundle/_expected/es6.js index def67b9..7d5460c 100644 --- a/test/form/self-contained-bundle/_expected/es6.js +++ b/test/form/self-contained-bundle/_expected/es6.js @@ -6,7 +6,6 @@ function bar () { return 42; } - // comment before 1 console.log( 1 ); diff --git a/test/form/self-contained-bundle/_expected/iife.js b/test/form/self-contained-bundle/_expected/iife.js index 1ec92be..46f6561 100644 --- a/test/form/self-contained-bundle/_expected/iife.js +++ b/test/form/self-contained-bundle/_expected/iife.js @@ -8,7 +8,6 @@ return 42; } - // comment before 1 console.log( 1 ); diff --git a/test/form/self-contained-bundle/_expected/umd.js b/test/form/self-contained-bundle/_expected/umd.js index 854c4e4..51b72b0 100644 --- a/test/form/self-contained-bundle/_expected/umd.js +++ b/test/form/self-contained-bundle/_expected/umd.js @@ -12,7 +12,6 @@ return 42; } - // comment before 1 console.log( 1 ); From 7cd858be602dae70ece424eb5763c079db7fb3cc Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 25 Jul 2015 14:41:57 -0400 Subject: [PATCH 04/11] failing tests - tricky case with unused declarators in multiple var declaration --- test/function/unused-var-a/_config.js | 3 +++ test/function/unused-var-a/foo.js | 2 ++ test/function/unused-var-a/main.js | 4 ++++ test/function/unused-var-b/_config.js | 3 +++ test/function/unused-var-b/foo.js | 2 ++ test/function/unused-var-b/main.js | 4 ++++ test/function/unused-var-c/_config.js | 3 +++ test/function/unused-var-c/foo.js | 2 ++ test/function/unused-var-c/main.js | 4 ++++ test/function/unused-var-d/_config.js | 3 +++ test/function/unused-var-d/foo.js | 5 +++++ test/function/unused-var-d/main.js | 4 ++++ 12 files changed, 39 insertions(+) create mode 100644 test/function/unused-var-a/_config.js create mode 100644 test/function/unused-var-a/foo.js create mode 100644 test/function/unused-var-a/main.js create mode 100644 test/function/unused-var-b/_config.js create mode 100644 test/function/unused-var-b/foo.js create mode 100644 test/function/unused-var-b/main.js create mode 100644 test/function/unused-var-c/_config.js create mode 100644 test/function/unused-var-c/foo.js create mode 100644 test/function/unused-var-c/main.js create mode 100644 test/function/unused-var-d/_config.js create mode 100644 test/function/unused-var-d/foo.js create mode 100644 test/function/unused-var-d/main.js diff --git a/test/function/unused-var-a/_config.js b/test/function/unused-var-a/_config.js new file mode 100644 index 0000000..05a2b74 --- /dev/null +++ b/test/function/unused-var-a/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'omits unused var declaration (a)' +}; diff --git a/test/function/unused-var-a/foo.js b/test/function/unused-var-a/foo.js new file mode 100644 index 0000000..ef10681 --- /dev/null +++ b/test/function/unused-var-a/foo.js @@ -0,0 +1,2 @@ +var unused = 'unused', foo = 'foo', bar = 'bar'; +export { foo, bar }; diff --git a/test/function/unused-var-a/main.js b/test/function/unused-var-a/main.js new file mode 100644 index 0000000..9922892 --- /dev/null +++ b/test/function/unused-var-a/main.js @@ -0,0 +1,4 @@ +import { foo, bar } from './foo'; + +assert.equal( foo, 'foo' ); +assert.equal( bar, 'bar' ); diff --git a/test/function/unused-var-b/_config.js b/test/function/unused-var-b/_config.js new file mode 100644 index 0000000..174d9fe --- /dev/null +++ b/test/function/unused-var-b/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'omits unused var declaration (b)' +}; diff --git a/test/function/unused-var-b/foo.js b/test/function/unused-var-b/foo.js new file mode 100644 index 0000000..26a8538 --- /dev/null +++ b/test/function/unused-var-b/foo.js @@ -0,0 +1,2 @@ +var foo = 'foo', unused = 'unused', bar = 'bar'; +export { foo, bar }; diff --git a/test/function/unused-var-b/main.js b/test/function/unused-var-b/main.js new file mode 100644 index 0000000..9922892 --- /dev/null +++ b/test/function/unused-var-b/main.js @@ -0,0 +1,4 @@ +import { foo, bar } from './foo'; + +assert.equal( foo, 'foo' ); +assert.equal( bar, 'bar' ); diff --git a/test/function/unused-var-c/_config.js b/test/function/unused-var-c/_config.js new file mode 100644 index 0000000..e40d968 --- /dev/null +++ b/test/function/unused-var-c/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'omits unused var declaration (c)' +}; diff --git a/test/function/unused-var-c/foo.js b/test/function/unused-var-c/foo.js new file mode 100644 index 0000000..14e28bb --- /dev/null +++ b/test/function/unused-var-c/foo.js @@ -0,0 +1,2 @@ +var foo = 'foo', bar = 'bar', unused = 'unused'; +export { foo, bar }; diff --git a/test/function/unused-var-c/main.js b/test/function/unused-var-c/main.js new file mode 100644 index 0000000..9922892 --- /dev/null +++ b/test/function/unused-var-c/main.js @@ -0,0 +1,4 @@ +import { foo, bar } from './foo'; + +assert.equal( foo, 'foo' ); +assert.equal( bar, 'bar' ); diff --git a/test/function/unused-var-d/_config.js b/test/function/unused-var-d/_config.js new file mode 100644 index 0000000..78379d6 --- /dev/null +++ b/test/function/unused-var-d/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'omits unused var declaration (d)' +}; diff --git a/test/function/unused-var-d/foo.js b/test/function/unused-var-d/foo.js new file mode 100644 index 0000000..630ff46 --- /dev/null +++ b/test/function/unused-var-d/foo.js @@ -0,0 +1,5 @@ +var unused_a = 'a', unused_b = 'b'; +var unused_c = 'c'; +var foo = 'foo', bar = 'bar'; + +export { foo, bar }; diff --git a/test/function/unused-var-d/main.js b/test/function/unused-var-d/main.js new file mode 100644 index 0000000..9922892 --- /dev/null +++ b/test/function/unused-var-d/main.js @@ -0,0 +1,4 @@ +import { foo, bar } from './foo'; + +assert.equal( foo, 'foo' ); +assert.equal( bar, 'bar' ); From 301f432b6f217cfea81d742fd5bc7505f97c29de Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 25 Jul 2015 14:47:26 -0400 Subject: [PATCH 05/11] remove unused code --- src/Bundle.js | 195 -------------------------------------------------- 1 file changed, 195 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index 85592f8..3bf0138 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -205,201 +205,6 @@ export default class Bundle { }); } - generate ( options = {} ) { - let magicString = new MagicString.Bundle({ separator: '' }); - - const format = options.format || 'es6'; - this.deconflict( format === 'es6' ); - - // If we have named exports from the bundle, and those exports - // are assigned to *within* the bundle, we may need to rewrite e.g. - // - // export let count = 0; - // export function incr () { count++ } - // - // might become... - // - // exports.count = 0; - // function incr () { - // exports.count += 1; - // } - // exports.incr = incr; - // - // This doesn't apply if the bundle is exported as ES6! - let allBundleExports = blank(); - - if ( format !== 'es6' ) { - keys( this.entryModule.exports ).forEach( key => { - const exportDeclaration = this.entryModule.exports[ key ]; - - const originalDeclaration = this.entryModule.findDeclaration( exportDeclaration.localName ); - - if ( originalDeclaration && originalDeclaration.type === 'VariableDeclaration' ) { - const canonicalName = this.entryModule.getCanonicalName( exportDeclaration.localName, false ); - - allBundleExports[ canonicalName ] = `exports.${key}`; - this.varExports[ key ] = true; - } - }); - } - - // since we're rewriting variable exports, we want to - // ensure we don't try and export them again at the bottom - this.toExport = keys( this.entryModule.exports ) - .filter( key => !this.varExports[ key ] ); - - // Apply new names and add to the output bundle - let previousModule = null; - let previousIndex = -1; - let previousMargin = 0; - - this.statements.forEach( statement => { - // skip `export { foo, bar, baz }` - if ( statement.node.type === 'ExportNamedDeclaration' ) { - // skip `export { foo, bar, baz }` - if ( statement.node.specifiers.length ) return; - - // skip `export var foo;` if foo is exported - if ( isEmptyExportedVarDeclaration( statement.node.declaration, statement.module, allBundleExports, format === 'es6' ) ) return; - } - - // skip empty var declarations for exported bindings - // (otherwise we're left with `exports.foo;`, which is useless) - if ( isEmptyExportedVarDeclaration( statement.node, statement.module, allBundleExports, format === 'es6' ) ) return; - - let replacements = blank(); - let bundleExports = blank(); - - keys( statement.dependsOn ) - .concat( keys( statement.defines ) ) - .forEach( name => { - const canonicalName = statement.module.getCanonicalName( name, format === 'es6' ); - - if ( allBundleExports[ canonicalName ] ) { - bundleExports[ name ] = replacements[ name ] = allBundleExports[ canonicalName ]; - } else if ( name !== canonicalName ) { - replacements[ name ] = canonicalName; - } - }); - - const source = statement.replaceIdentifiers( replacements, bundleExports ); - - // modify exports as necessary - if ( statement.isExportDeclaration ) { - // remove `export` from `export var foo = 42` - if ( statement.node.type === 'ExportNamedDeclaration' && statement.node.declaration.type === 'VariableDeclaration' ) { - source.remove( statement.node.start, statement.node.declaration.start ); - } - - // remove `export` from `export class Foo {...}` or `export default Foo` - // TODO default exports need different treatment - else if ( statement.node.declaration.id ) { - source.remove( statement.node.start, statement.node.declaration.start ); - } - - else if ( statement.node.type === 'ExportDefaultDeclaration' ) { - const module = statement.module; - const canonicalName = module.getCanonicalName( 'default', format === 'es6' ); - - if ( statement.node.declaration.type === 'Identifier' && canonicalName === module.getCanonicalName( statement.node.declaration.name, format === 'es6' ) ) { - return; - } - - // anonymous functions should be converted into declarations - if ( statement.node.declaration.type === 'FunctionExpression' ) { - source.overwrite( statement.node.start, statement.node.declaration.start + 8, `function ${canonicalName}` ); - } else { - source.overwrite( statement.node.start, statement.node.declaration.start, `var ${canonicalName} = ` ); - } - } - - else { - throw new Error( 'Unhandled export' ); - } - } - - // ensure there is always a newline between statements, and add - // additional newlines as necessary to reflect original source - const minSeparation = ( previousModule !== statement.module ) || ( statement.index !== previousIndex + 1 ) ? 3 : 2; - const margin = Math.max( minSeparation, statement.margin[0], previousMargin ); - let newLines = new Array( margin ).join( '\n' ); - - // add leading comments - if ( statement.leadingComments.length ) { - const commentBlock = newLines + statement.leadingComments.map( ({ separator, comment }) => { - return separator + ( comment.block ? - `/*${comment.text}*/` : - `//${comment.text}` ); - }).join( '' ); - - magicString.addSource( new MagicString( commentBlock ) ); - newLines = new Array( statement.margin[0] ).join( '\n' ); // TODO handle gaps between comment block and statement - } - - // add the statement itself - magicString.addSource({ - content: source, - separator: newLines - }); - - // add trailing comments - const comment = statement.trailingComment; - if ( comment ) { - const commentBlock = comment.block ? - ` /*${comment.text}*/` : - ` //${comment.text}`; - - magicString.append( commentBlock ); - } - - previousMargin = statement.margin[1]; - previousModule = statement.module; - previousIndex = statement.index; - }); - - // prepend bundle with internal namespaces - const indentString = magicString.getIndentString(); - const namespaceBlock = this.internalNamespaceModules.map( module => { - const exportKeys = keys( module.exports ); - - return `var ${module.getCanonicalName('*', format === 'es6')} = {\n` + - exportKeys.map( key => `${indentString}get ${key} () { return ${module.getCanonicalName(key, format === 'es6')}; }` ).join( ',\n' ) + - `\n};\n\n`; - }).join( '' ); - - magicString.prepend( namespaceBlock ); - - const finalise = finalisers[ format ]; - - if ( !finalise ) { - throw new Error( `You must specify an output type - valid options are ${keys( finalisers ).join( ', ' )}` ); - } - - magicString = finalise( this, magicString.trim(), { - // Determine export mode - 'default', 'named', 'none' - exportMode: getExportMode( this, options.exports ), - - // Determine indentation - indentString: getIndentString( magicString, options ) - }, options ); - - const code = magicString.toString(); - let map = null; - - if ( options.sourceMap ) { - const file = options.sourceMapFile || options.dest; - map = magicString.generateMap({ - includeContent: true, - file - // TODO - }); - - map.sources = map.sources.map( unixizePath ); - } - - return { code, map }; - } - markAllModifierStatements () { let settled = true; let promises = []; From cbae9fc8de42ff7f1d0e69d45bd33711c28b99d1 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 25 Jul 2015 14:48:29 -0400 Subject: [PATCH 06/11] remove more unused code --- src/Module.js | 37 ------------------------------------- 1 file changed, 37 deletions(-) diff --git a/src/Module.js b/src/Module.js index 831a748..5db2986 100644 --- a/src/Module.js +++ b/src/Module.js @@ -691,43 +691,6 @@ export default class Module { throw new Error( 'Unhandled export' ); } } - - // // ensure there is always a newline between statements, and add - // // additional newlines as necessary to reflect original source - // const minSeparation = ( statement.index !== previousIndex + 1 ) ? 3 : 2; - // const margin = Math.max( minSeparation, statement.margin[0], previousMargin ); - // let newLines = new Array( margin ).join( '\n' ); - // - // // add leading comments - // if ( statement.leadingComments.length ) { - // const commentBlock = newLines + statement.leadingComments.map( ({ separator, comment }) => { - // return separator + ( comment.block ? - // `/*${comment.text}*/` : - // `//${comment.text}` ); - // }).join( '' ); - // - // magicString.addSource( new MagicString( commentBlock ) ); - // newLines = new Array( statement.margin[0] ).join( '\n' ); // TODO handle gaps between comment block and statement - // } - // - // // add the statement itself - // magicString.addSource({ - // content: source, - // separator: newLines - // }); - // - // // add trailing comments - // const comment = statement.trailingComment; - // if ( comment ) { - // const commentBlock = comment.block ? - // ` /*${comment.text}*/` : - // ` //${comment.text}`; - // - // magicString.append( commentBlock ); - // } - // - // previousMargin = statement.margin[1]; - // previousIndex = statement.index; }); return magicString.trim(); From 9c22bc12c6d7f2c43fef5fb194760929f73458c5 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 25 Jul 2015 19:09:12 -0400 Subject: [PATCH 07/11] only insert export initialisers if there are any --- src/Statement.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Statement.js b/src/Statement.js index 9012d46..bad15b6 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -286,11 +286,13 @@ export default class Statement { .map( name => `\n${bundleExports[name]} = ${name};` ) .join( '' ); - // TODO clean this up - try { - magicString.insert( node.end, exportInitialisers ); - } catch ( err ) { - magicString.append( exportInitialisers ); + if ( exportInitialisers ) { + // TODO clean this up + try { + magicString.insert( node.end, exportInitialisers ); + } catch ( err ) { + magicString.append( exportInitialisers ); + } } } } From 24d061450a173bbd4511a563041360f9b4831dbd Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 25 Jul 2015 19:12:18 -0400 Subject: [PATCH 08/11] fix variable splitting --- package.json | 2 +- src/Module.js | 29 +++++++++++++++-------------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index b5f55c2..fcc43da 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "dependencies": { "acorn": "^1.1.0", "chalk": "^1.0.0", - "magic-string": "^0.6.2", + "magic-string": "^0.6.4", "minimist": "^1.1.1", "sander": "^0.3.3", "source-map-support": "^0.3.1" diff --git a/src/Module.js b/src/Module.js index 5db2986..6e38164 100644 --- a/src/Module.js +++ b/src/Module.js @@ -547,27 +547,22 @@ export default class Module { // should be split up. Otherwise, we may end up including code we // don't need, just because an unwanted declarator is included if ( node.type === 'VariableDeclaration' && node.declarations.length > 1 ) { + // remove the leading var/let/const + this.magicString.remove( node.start, node.declarations[0].start ); node.declarations.forEach( ( declarator, i ) => { - //const magicString = this.magicString.snip( declarator.start, declarator.end ).trim(); - const nextDeclarator = node.declarations[ i + 1 ]; - - // if ( nextDeclarator ) { - // this.magicString.overwrite( declarator.end, nextDeclarator.start, `;\n${node.kind} ` ); // TODO indentation - // } + const { start, end } = declarator; const syntheticNode = { type: 'VariableDeclaration', kind: node.kind, - start: node.start, - end: node.end, - declarations: [ declarator ] + start, + end, + declarations: [ declarator ], + isSynthetic: true }; - const start = i === 0 ? node.start : declarator.start; - const end = declarator.end; - - const statement = new Statement( syntheticNode, this, start, end ); // TODO this is almost certainly wrong... + const statement = new Statement( syntheticNode, this, start, end ); statements.push( statement ); }); @@ -612,7 +607,7 @@ export default class Module { let previousIndex = -1; let previousMargin = 0; - this.statements.forEach( statement => { + this.statements.forEach( ( statement, i ) => { if ( !statement.isIncluded ) { magicString.remove( statement.start, statement.next ); return; @@ -640,6 +635,12 @@ export default class Module { return; } + // split up/remove var declarations as necessary + if ( statement.node.isSynthetic ) { + magicString.insert( statement.start, `${statement.node.kind} ` ); + magicString.overwrite( statement.end, statement.next, ';\n' ); // TODO account for trailing newlines + } + let replacements = blank(); let bundleExports = blank(); From acdfbead47b8d5399b2b5a5cbbdf0a956fe3516c Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 25 Jul 2015 19:12:28 -0400 Subject: [PATCH 09/11] update test --- test/form/exported-empty-vars/_expected/es6.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/form/exported-empty-vars/_expected/es6.js b/test/form/exported-empty-vars/_expected/es6.js index 8c8cdc3..3be564f 100644 --- a/test/form/exported-empty-vars/_expected/es6.js +++ b/test/form/exported-empty-vars/_expected/es6.js @@ -1,8 +1,8 @@ var foo; foo = 42; -var bar, baz; - +var bar; +var baz; bar = 43; baz = 44; From 7c1d2bb8e679e940940c42e65391a8a2ddeb5dd6 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 25 Jul 2015 20:58:01 -0400 Subject: [PATCH 10/11] remove unused code --- src/Module.js | 7 ++--- src/Statement.js | 4 --- src/ast/analyse.js | 66 ---------------------------------------------- 3 files changed, 2 insertions(+), 75 deletions(-) delete mode 100644 src/ast/analyse.js diff --git a/src/Module.js b/src/Module.js index 6e38164..ee71c8f 100644 --- a/src/Module.js +++ b/src/Module.js @@ -4,7 +4,6 @@ import { parse } from 'acorn'; import MagicString from 'magic-string'; import Statement from './Statement'; import walk from './ast/walk'; -import analyse from './ast/analyse'; import { blank, keys } from './utils/object'; import { first, sequence } from './utils/promise'; import { isImportDeclaration, isExportDeclaration } from './utils/map-helpers'; @@ -180,12 +179,10 @@ export default class Module { this.statements.forEach( statement => { if ( isImportDeclaration( statement ) ) this.addImport( statement ); else if ( isExportDeclaration( statement ) ) this.addExport( statement ); - }); - analyse( this.magicString, this ); + statement.analyse(); - // consolidate names that are defined/modified in this module - this.statements.forEach( statement => { + // consolidate names that are defined/modified in this module keys( statement.defines ).forEach( name => { this.definitions[ name ] = statement; }); diff --git a/src/Statement.js b/src/Statement.js index bad15b6..1ed36d7 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -24,10 +24,6 @@ export default class Statement { this.isIncluded = false; - this.leadingComments = []; - this.trailingComment = null; - this.margin = [ 0, 0 ]; - // some facts about this statement... this.isImportDeclaration = node.type === 'ImportDeclaration'; this.isExportDeclaration = /^Export/.test( node.type ); diff --git a/src/ast/analyse.js b/src/ast/analyse.js deleted file mode 100644 index 21b5f4c..0000000 --- a/src/ast/analyse.js +++ /dev/null @@ -1,66 +0,0 @@ -export default function analyse ( magicString, module ) { - // first we need to generate comprehensive scope info - let previousStatement = null; - let commentIndex = 0; - - module.statements.forEach( statement => { - const node = statement.node; - - let trailing = !!previousStatement; - let previousComment; - - // TODO surely this can be neater - // attach leading comment - do { - let comment = module.comments[ commentIndex ]; - - // prevent comments inside the previous statement being - // appended to it - if ( previousStatement ) { - while ( comment && comment.start < previousStatement.node.end ) { - commentIndex += 1; - comment = module.comments[ commentIndex ]; - } - } - - if ( !comment || ( comment.end > node.start ) ) break; - - // attach any trailing comment to the previous statement - if ( trailing && !/\n/.test( module.source.slice( previousStatement.node.end, comment.start ) ) ) { - previousStatement.trailingComment = comment; - } - - // then attach leading comments to this statement - else { - statement.leadingComments.push({ - separator: previousComment ? magicString.slice( previousComment.end, comment.start ) : '\n', - comment - }); - - previousComment = comment; - } - - commentIndex += 1; - trailing = false; - } while ( module.comments[ commentIndex ] ); - - // determine margin - const previousEnd = previousComment ? - previousComment.end : - previousStatement ? - ( previousStatement.trailingComment || previousStatement.node ).end : - 0; - - //const start = ( statement.leadingComments[0] || node ).start; - - const gap = magicString.original.slice( previousEnd, node.start ); - const margin = gap.split( '\n' ).length; - - if ( previousStatement ) previousStatement.margin[1] = margin; - statement.margin[0] = margin; - - statement.analyse(); - - previousStatement = statement; - }); -} From 405bc00a56dbef4901a9615ed98294eef2c85792 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 25 Jul 2015 21:02:56 -0400 Subject: [PATCH 11/11] more tidying up --- src/Module.js | 5 ++--- src/Statement.js | 2 -- src/utils/map-helpers.js | 8 -------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/Module.js b/src/Module.js index ee71c8f..3cf9b8a 100644 --- a/src/Module.js +++ b/src/Module.js @@ -6,7 +6,6 @@ import Statement from './Statement'; import walk from './ast/walk'; import { blank, keys } from './utils/object'; import { first, sequence } from './utils/promise'; -import { isImportDeclaration, isExportDeclaration } from './utils/map-helpers'; import getLocation from './utils/getLocation'; import makeLegalIdentifier from './utils/makeLegalIdentifier'; @@ -177,8 +176,8 @@ export default class Module { analyse () { // discover this module's imports and exports this.statements.forEach( statement => { - if ( isImportDeclaration( statement ) ) this.addImport( statement ); - else if ( isExportDeclaration( statement ) ) this.addExport( statement ); + if ( statement.isImportDeclaration ) this.addImport( statement ); + else if ( statement.isExportDeclaration ) this.addExport( statement ); statement.analyse(); diff --git a/src/Statement.js b/src/Statement.js index 1ed36d7..43b6bf7 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -24,10 +24,8 @@ export default class Statement { this.isIncluded = false; - // some facts about this statement... this.isImportDeclaration = node.type === 'ImportDeclaration'; this.isExportDeclaration = /^Export/.test( node.type ); - this.isExportAllDeclaration = /^ExportAll/.test( node.type ); } analyse () { diff --git a/src/utils/map-helpers.js b/src/utils/map-helpers.js index 9576535..49e0942 100644 --- a/src/utils/map-helpers.js +++ b/src/utils/map-helpers.js @@ -9,11 +9,3 @@ export function quoteId ( x ) { export function req ( x ) { return `require('${x.id}')`; } - -export function isImportDeclaration ( statement ) { - return statement.isImportDeclaration; -} - -export function isExportDeclaration ( statement ) { - return statement.isExportDeclaration; -}