diff --git a/src/Bundle.js b/src/Bundle.js index ffb3fc2..25cff8a 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -290,13 +290,13 @@ export default class Bundle { // // This doesn't apply if the bundle is exported as ES6! let allBundleExports = blank(); - let isVarDeclaration = blank(); + let isReassignedVarDeclaration = blank(); let varExports = blank(); let getterExports = []; this.orderedModules.forEach( module => { - module.varDeclarations.forEach( name => { - isVarDeclaration[ module.replacements[ name ] || name ] = true; + module.reassignments.forEach( name => { + isReassignedVarDeclaration[ module.replacements[ name ] || name ] = true; }); }); @@ -306,7 +306,7 @@ export default class Bundle { .forEach( name => { const canonicalName = this.traceExport( this.entryModule, name ); - if ( isVarDeclaration[ canonicalName ] ) { + if ( isReassignedVarDeclaration[ canonicalName ] ) { varExports[ name ] = true; // if the same binding is exported multiple ways, we need to diff --git a/src/Module.js b/src/Module.js index 1e79c68..384fa7d 100644 --- a/src/Module.js +++ b/src/Module.js @@ -63,7 +63,7 @@ export default class Module { this.replacements = blank(); - this.varDeclarations = []; + this.reassignments = []; this.marked = blank(); this.definitions = blank(); @@ -206,20 +206,35 @@ export default class Module { this.definitions[ name ] = statement; }); - statement.scope.varDeclarations.forEach( name => { - this.varDeclarations.push( name ); - }); - keys( statement.modifies ).forEach( name => { ( this.modifications[ name ] || ( this.modifications[ name ] = [] ) ).push( statement ); }); }); + // discover variables that are reassigned inside function + // bodies, so we can keep bindings live, e.g. + // + // export var count = 0; + // export function incr () { count += 1 } + let reassigned = blank(); + this.statements.forEach( statement => { + keys( statement.reassigns ).forEach( name => { + reassigned[ name ] = true; + }); + }); + // if names are referenced that are neither defined nor imported // in this module, we assume that they're globals this.statements.forEach( statement => { if ( statement.isReexportDeclaration ) return; + // while we're here, mark reassignments + statement.scope.varDeclarations.forEach( name => { + if ( reassigned[ name ] && !~this.reassignments.indexOf( name ) ) { + this.reassignments.push( name ); + } + }); + keys( statement.dependsOn ).forEach( name => { if ( !this.definitions[ name ] && !this.imports[ name ] ) { this.bundle.assumedGlobals[ name ] = true; diff --git a/src/Statement.js b/src/Statement.js index 27b70d8..1a98a5e 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -29,6 +29,8 @@ export default class Statement { this.dependsOn = blank(); this.stronglyDependsOn = blank(); + this.reassigns = blank(); + this.isIncluded = false; this.isImportDeclaration = node.type === 'ImportDeclaration'; @@ -46,29 +48,23 @@ export default class Statement { let newScope; switch ( node.type ) { - case 'FunctionExpression': case 'FunctionDeclaration': - case 'ArrowFunctionExpression': - if ( node.type === 'FunctionDeclaration' ) { - scope.addDeclaration( node.id.name, node, false ); - } - - newScope = new Scope({ - parent: scope, - params: node.params, // TODO rest params? - block: false - }); - - // named function expressions - the name is considered - // part of the function's scope - if ( node.type === 'FunctionExpression' && node.id ) { - newScope.addDeclaration( node.id.name, node, false ); - } - - break; + scope.addDeclaration( node.id.name, node, false ); case 'BlockStatement': - if ( !/Function/.test( parent.type ) ) { + if ( parent && /Function/.test( parent.type ) ) { + newScope = new Scope({ + parent: scope, + block: false, + params: parent.params + }); + + // named function expressions - the name is considered + // part of the function's scope + if ( parent.type === 'FunctionExpression' && parent.id ) { + newScope.addDeclaration( parent.id.name, parent, false ); + } + } else { newScope = new Scope({ parent: scope, block: true @@ -133,27 +129,19 @@ export default class Statement { if ( !this.isImportDeclaration ) { walk( this.node, { enter: ( node, parent ) => { - if ( node._scope ) { - if ( !scope.isBlockScope ) { - if ( !isIife( node, parent ) ) readDepth += 1; - if ( isFunctionDeclaration( node, parent ) ) writeDepth += 1; - } + if ( isFunctionDeclaration( node, parent ) ) writeDepth += 1; + if ( /Function/.test( node.type ) && !isIife( node, parent ) ) readDepth += 1; - scope = node._scope; - } + if ( node._scope ) scope = node._scope; this.checkForReads( scope, node, parent, !readDepth ); this.checkForWrites( scope, node, writeDepth ); }, leave: ( node, parent ) => { - if ( node._scope ) { - if ( !scope.isBlockScope ) { - if ( !isIife( node, parent ) ) readDepth -= 1; - if ( isFunctionDeclaration( node, parent ) ) writeDepth -= 1; - } + if ( isFunctionDeclaration( node, parent ) ) writeDepth -= 1; + if ( /Function/.test( node.type ) && !isIife( node, parent ) ) readDepth -= 1; - scope = scope.parent; - } + if ( node._scope ) scope = scope.parent; } }); } @@ -226,6 +214,17 @@ export default class Statement { this.module.exports.default.isModified = true; } } + + // we track updates/reassignments to variables, to know whether we + // need to rewrite it later from `foo` to `exports.foo` to keep + // bindings live + if ( + depth === 0 && + writeDepth > 0 && + !scope.contains( node.name ) + ) { + this.reassigns[ node.name ] = true; + } } // we only care about writes that happen a) at the top level, @@ -348,11 +347,6 @@ export default class Statement { let newNames = blank(); let hasReplacements; - // special case = function foo ( foo ) {...} - if ( node.id && names[ node.id.name ] && scope.declarations[ node.id.name ] ) { - magicString.overwrite( node.id.start, node.id.end, names[ node.id.name ] ); - } - keys( names ).forEach( name => { if ( !scope.declarations[ name ] ) { newNames[ name ] = names[ name ]; @@ -393,6 +387,8 @@ export default class Statement { if ( parent.type === 'MemberExpression' && !parent.computed && node !== parent.object ) return; if ( parent.type === 'Property' && node !== parent.value ) return; if ( parent.type === 'MethodDefinition' && node === parent.key ) return; + if ( parent.type === 'FunctionExpression' ) return; + if ( /Function/.test( parent.type ) && ~parent.params.indexOf( node ) ) return; // TODO others...? // all other identifiers should be overwritten diff --git a/test/form/exported-empty-vars/_config.js b/test/form/exported-empty-vars/_config.js deleted file mode 100644 index fa0287c..0000000 --- a/test/form/exported-empty-vars/_config.js +++ /dev/null @@ -1,6 +0,0 @@ -module.exports = { - description: 'removes empty var declarations that are exported', - options: { - moduleName: 'myBundle' - } -}; diff --git a/test/form/exported-empty-vars/_expected/amd.js b/test/form/exported-empty-vars/_expected/amd.js deleted file mode 100644 index c0f1472..0000000 --- a/test/form/exported-empty-vars/_expected/amd.js +++ /dev/null @@ -1,8 +0,0 @@ -define(['exports'], function (exports) { 'use strict'; - - exports.foo = 42; - - exports.bar = 43; - exports.baz = 44; - -}); diff --git a/test/form/exported-empty-vars/_expected/cjs.js b/test/form/exported-empty-vars/_expected/cjs.js deleted file mode 100644 index 4011bd7..0000000 --- a/test/form/exported-empty-vars/_expected/cjs.js +++ /dev/null @@ -1,6 +0,0 @@ -'use strict'; - -exports.foo = 42; - -exports.bar = 43; -exports.baz = 44; diff --git a/test/form/exported-empty-vars/_expected/es6.js b/test/form/exported-empty-vars/_expected/es6.js deleted file mode 100644 index 4b76753..0000000 --- a/test/form/exported-empty-vars/_expected/es6.js +++ /dev/null @@ -1,9 +0,0 @@ -var foo; -foo = 42; - -var bar; -var baz; -bar = 43; -baz = 44; - -export { foo, bar, baz }; diff --git a/test/form/exported-empty-vars/_expected/iife.js b/test/form/exported-empty-vars/_expected/iife.js deleted file mode 100644 index 63c56b2..0000000 --- a/test/form/exported-empty-vars/_expected/iife.js +++ /dev/null @@ -1,8 +0,0 @@ -(function (exports) { 'use strict'; - - exports.foo = 42; - - exports.bar = 43; - exports.baz = 44; - -})((this.myBundle = {})); diff --git a/test/form/exported-empty-vars/bar.js b/test/form/exported-empty-vars/bar.js deleted file mode 100644 index b8818da..0000000 --- a/test/form/exported-empty-vars/bar.js +++ /dev/null @@ -1,6 +0,0 @@ -var bar, baz; - -bar = 43; -baz = 44; - -export { bar, baz }; diff --git a/test/form/exported-empty-vars/foo.js b/test/form/exported-empty-vars/foo.js deleted file mode 100644 index 2c71d30..0000000 --- a/test/form/exported-empty-vars/foo.js +++ /dev/null @@ -1,2 +0,0 @@ -export var foo; -foo = 42; diff --git a/test/form/exported-empty-vars/main.js b/test/form/exported-empty-vars/main.js deleted file mode 100644 index 21ee721..0000000 --- a/test/form/exported-empty-vars/main.js +++ /dev/null @@ -1,2 +0,0 @@ -export { foo } from './foo'; -export { bar, baz } from './bar'; diff --git a/test/form/exports-at-end-if-possible/_config.js b/test/form/exports-at-end-if-possible/_config.js new file mode 100644 index 0000000..a898bc7 --- /dev/null +++ b/test/form/exports-at-end-if-possible/_config.js @@ -0,0 +1,7 @@ +module.exports = { + description: 'exports variables at end, if possible', + options: { + moduleName: 'myBundle' + }, + // solo: true +}; diff --git a/test/form/exports-at-end-if-possible/_expected/amd.js b/test/form/exports-at-end-if-possible/_expected/amd.js new file mode 100644 index 0000000..2612901 --- /dev/null +++ b/test/form/exports-at-end-if-possible/_expected/amd.js @@ -0,0 +1,11 @@ +define(['exports'], function (exports) { 'use strict'; + + var FOO = 'foo'; + + console.log( FOO ); + console.log( FOO ); + console.log( FOO ); + + exports.FOO = FOO; + +}); diff --git a/test/form/exports-at-end-if-possible/_expected/cjs.js b/test/form/exports-at-end-if-possible/_expected/cjs.js new file mode 100644 index 0000000..57d55d3 --- /dev/null +++ b/test/form/exports-at-end-if-possible/_expected/cjs.js @@ -0,0 +1,9 @@ +'use strict'; + +var FOO = 'foo'; + +console.log( FOO ); +console.log( FOO ); +console.log( FOO ); + +exports.FOO = FOO; diff --git a/test/form/exports-at-end-if-possible/_expected/es6.js b/test/form/exports-at-end-if-possible/_expected/es6.js new file mode 100644 index 0000000..b42b4e4 --- /dev/null +++ b/test/form/exports-at-end-if-possible/_expected/es6.js @@ -0,0 +1,7 @@ +var FOO = 'foo'; + +console.log( FOO ); +console.log( FOO ); +console.log( FOO ); + +export { FOO }; diff --git a/test/form/exports-at-end-if-possible/_expected/iife.js b/test/form/exports-at-end-if-possible/_expected/iife.js new file mode 100644 index 0000000..d1b29c8 --- /dev/null +++ b/test/form/exports-at-end-if-possible/_expected/iife.js @@ -0,0 +1,11 @@ +(function (exports) { 'use strict'; + + var FOO = 'foo'; + + console.log( FOO ); + console.log( FOO ); + console.log( FOO ); + + exports.FOO = FOO; + +})((this.myBundle = {})); diff --git a/test/form/exported-empty-vars/_expected/umd.js b/test/form/exports-at-end-if-possible/_expected/umd.js similarity index 72% rename from test/form/exported-empty-vars/_expected/umd.js rename to test/form/exports-at-end-if-possible/_expected/umd.js index 9cde4bd..f30f365 100644 --- a/test/form/exported-empty-vars/_expected/umd.js +++ b/test/form/exports-at-end-if-possible/_expected/umd.js @@ -4,9 +4,12 @@ factory((global.myBundle = {})); }(this, function (exports) { 'use strict'; - exports.foo = 42; + var FOO = 'foo'; - exports.bar = 43; - exports.baz = 44; + console.log( FOO ); + console.log( FOO ); + console.log( FOO ); + + exports.FOO = FOO; })); diff --git a/test/form/exports-at-end-if-possible/main.js b/test/form/exports-at-end-if-possible/main.js new file mode 100644 index 0000000..ee6a75a --- /dev/null +++ b/test/form/exports-at-end-if-possible/main.js @@ -0,0 +1,5 @@ +export var FOO = 'foo'; + +console.log( FOO ); +console.log( FOO ); +console.log( FOO ); diff --git a/test/form/multiple-exports/_expected/amd.js b/test/form/multiple-exports/_expected/amd.js index 5460f4f..2ebff3b 100644 --- a/test/form/multiple-exports/_expected/amd.js +++ b/test/form/multiple-exports/_expected/amd.js @@ -1,6 +1,9 @@ define(['exports'], function (exports) { 'use strict'; - exports.foo = 1; - exports.bar = 2; + var foo = 1; + var bar = 2; + + exports.foo = foo; + exports.bar = bar; }); diff --git a/test/form/multiple-exports/_expected/cjs.js b/test/form/multiple-exports/_expected/cjs.js index 0bbfe60..1968b8b 100644 --- a/test/form/multiple-exports/_expected/cjs.js +++ b/test/form/multiple-exports/_expected/cjs.js @@ -1,4 +1,7 @@ 'use strict'; -exports.foo = 1; -exports.bar = 2; +var foo = 1; +var bar = 2; + +exports.foo = foo; +exports.bar = bar; diff --git a/test/form/multiple-exports/_expected/iife.js b/test/form/multiple-exports/_expected/iife.js index 09461cb..8ed8290 100644 --- a/test/form/multiple-exports/_expected/iife.js +++ b/test/form/multiple-exports/_expected/iife.js @@ -1,6 +1,9 @@ (function (exports) { 'use strict'; - exports.foo = 1; - exports.bar = 2; + var foo = 1; + var bar = 2; + + exports.foo = foo; + exports.bar = bar; })((this.myBundle = {})); diff --git a/test/form/multiple-exports/_expected/umd.js b/test/form/multiple-exports/_expected/umd.js index 2fd0eb7..687b75c 100644 --- a/test/form/multiple-exports/_expected/umd.js +++ b/test/form/multiple-exports/_expected/umd.js @@ -4,7 +4,10 @@ factory((global.myBundle = {})); }(this, function (exports) { 'use strict'; - exports.foo = 1; - exports.bar = 2; + var foo = 1; + var bar = 2; + + exports.foo = foo; + exports.bar = bar; })); diff --git a/test/form/preserves-comments-after-imports/_expected/amd.js b/test/form/preserves-comments-after-imports/_expected/amd.js index fd4591b..82a0a52 100644 --- a/test/form/preserves-comments-after-imports/_expected/amd.js +++ b/test/form/preserves-comments-after-imports/_expected/amd.js @@ -4,6 +4,8 @@ define(['exports'], function (exports) { 'use strict'; var number = 5; /** A comment for obj */ - exports.obj = { number }; + var obj = { number }; + + exports.obj = obj; }); diff --git a/test/form/preserves-comments-after-imports/_expected/cjs.js b/test/form/preserves-comments-after-imports/_expected/cjs.js index 86e64c1..8b15cd1 100644 --- a/test/form/preserves-comments-after-imports/_expected/cjs.js +++ b/test/form/preserves-comments-after-imports/_expected/cjs.js @@ -4,4 +4,6 @@ var number = 5; /** A comment for obj */ -exports.obj = { number }; +var obj = { number }; + +exports.obj = obj; diff --git a/test/form/preserves-comments-after-imports/_expected/iife.js b/test/form/preserves-comments-after-imports/_expected/iife.js index 02f5255..9da8252 100644 --- a/test/form/preserves-comments-after-imports/_expected/iife.js +++ b/test/form/preserves-comments-after-imports/_expected/iife.js @@ -4,6 +4,8 @@ var number = 5; /** A comment for obj */ - exports.obj = { number }; + var obj = { number }; + + exports.obj = obj; })((this.myBundle = {})); diff --git a/test/form/preserves-comments-after-imports/_expected/umd.js b/test/form/preserves-comments-after-imports/_expected/umd.js index 6993070..96fec25 100644 --- a/test/form/preserves-comments-after-imports/_expected/umd.js +++ b/test/form/preserves-comments-after-imports/_expected/umd.js @@ -8,6 +8,8 @@ var number = 5; /** A comment for obj */ - exports.obj = { number }; + var obj = { number }; + + exports.obj = obj; })); diff --git a/test/function/assignment-to-exports/_config.js b/test/function/assignment-to-exports/_config.js index c958adb..fac0cb3 100644 --- a/test/function/assignment-to-exports/_config.js +++ b/test/function/assignment-to-exports/_config.js @@ -6,5 +6,6 @@ module.exports = { assert.equal( exports.count, 0 ); exports.incr(); assert.equal( exports.count, 1 ); - } + }, + // solo: true }; diff --git a/test/function/functions-renamed-correctly/_config.js b/test/function/functions-renamed-correctly/_config.js new file mode 100644 index 0000000..b0719a0 --- /dev/null +++ b/test/function/functions-renamed-correctly/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'renames function expression IDs correctly' +}; diff --git a/test/function/functions-renamed-correctly/after.js b/test/function/functions-renamed-correctly/after.js new file mode 100644 index 0000000..549d2db --- /dev/null +++ b/test/function/functions-renamed-correctly/after.js @@ -0,0 +1,5 @@ +function x () { + return 'after'; +} + +export { x as after }; diff --git a/test/function/functions-renamed-correctly/before.js b/test/function/functions-renamed-correctly/before.js new file mode 100644 index 0000000..56fba5f --- /dev/null +++ b/test/function/functions-renamed-correctly/before.js @@ -0,0 +1,5 @@ +function x () { + return 'before'; +} + +export { x as before }; diff --git a/test/function/functions-renamed-correctly/factorial.js b/test/function/functions-renamed-correctly/factorial.js new file mode 100644 index 0000000..9f5e752 --- /dev/null +++ b/test/function/functions-renamed-correctly/factorial.js @@ -0,0 +1,7 @@ +var x = (function () { + return function x ( num ) { + return num <= 2 ? num : num * x( num - 1 ); + }; +})(); + +export { x }; diff --git a/test/function/functions-renamed-correctly/main.js b/test/function/functions-renamed-correctly/main.js new file mode 100644 index 0000000..d2e616a --- /dev/null +++ b/test/function/functions-renamed-correctly/main.js @@ -0,0 +1,7 @@ +import { before } from './before'; +import { x } from './factorial'; +import { after } from './after'; + +before(); // before and after ensure x is renamed +assert.equal( x( 5 ), 120 ); +after();