From dcb11478867ff407b3feeea0bf057944e55fd92c Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 19 Jul 2015 10:05:21 -0400 Subject: [PATCH 1/6] exclude unnecessary functions containing assignments/updates to included names (#37) --- src/Statement.js | 40 +++++++++++++++---- .../_config.js | 3 ++ .../_expected/amd.js | 18 +++++++++ .../_expected/cjs.js | 16 ++++++++ .../_expected/es6.js | 14 +++++++ .../_expected/iife.js | 18 +++++++++ .../_expected/umd.js | 22 ++++++++++ .../exclude-unnecessary-modifications/foo.js | 27 +++++++++++++ .../exclude-unnecessary-modifications/main.js | 2 + 9 files changed, 153 insertions(+), 7 deletions(-) create mode 100644 test/form/exclude-unnecessary-modifications/_config.js create mode 100644 test/form/exclude-unnecessary-modifications/_expected/amd.js create mode 100644 test/form/exclude-unnecessary-modifications/_expected/cjs.js create mode 100644 test/form/exclude-unnecessary-modifications/_expected/es6.js create mode 100644 test/form/exclude-unnecessary-modifications/_expected/iife.js create mode 100644 test/form/exclude-unnecessary-modifications/_expected/umd.js create mode 100644 test/form/exclude-unnecessary-modifications/foo.js create mode 100644 test/form/exclude-unnecessary-modifications/main.js diff --git a/src/Statement.js b/src/Statement.js index 7cfbebf..9b4eb90 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -8,6 +8,14 @@ function isIife ( node, parent ) { return parent && parent.type === 'CallExpression' && node === parent.callee; } +function isFunctionDeclaration ( node, parent ) { + // `function foo () {}` + if ( node.type === 'FunctionDeclaration' ) return true; + + // `var foo = function () {}` - same thing for present purposes + if ( node.type === 'FunctionExpression' && parent.type === 'VariableDeclarator' ) return true; +} + export default class Statement { constructor ( node, magicString, module, index ) { this.node = node; @@ -123,22 +131,34 @@ export default class Statement { // calledImmediately(); // // ...but it's better than nothing - let depth = 0; + let readDepth = 0; + + // This allows us to track whether a modifying statement (i.e. assignment + // /update expressions) need to be captured + let writeDepth = 0; if ( !this.isImportDeclaration ) { walk( this.node, { enter: ( node, parent ) => { if ( node._scope ) { - if ( !scope.isBlockScope && !isIife( node, parent ) ) depth += 1; + if ( !scope.isBlockScope ) { + if ( !isIife( node, parent ) ) readDepth += 1; + if ( isFunctionDeclaration( node, parent ) ) writeDepth += 1; + } + scope = node._scope; } - this.checkForReads( scope, node, parent, !depth ); - this.checkForWrites( scope, node ); + this.checkForReads( scope, node, parent, !readDepth ); + this.checkForWrites( scope, node, writeDepth ); }, leave: ( node, parent ) => { if ( node._scope ) { - if ( !scope.isBlockScope && !isIife( node, parent ) ) depth -= 1; + if ( !scope.isBlockScope ) { + if ( !isIife( node, parent ) ) readDepth -= 1; + if ( isFunctionDeclaration( node, parent ) ) writeDepth -= 1; + } + scope = scope.parent; } } @@ -174,7 +194,7 @@ export default class Statement { } } - checkForWrites ( scope, node ) { + checkForWrites ( scope, node, writeDepth ) { const addNode = ( node, isAssignment ) => { let depth = 0; // determine whether we're illegally modifying a binding or namespace @@ -212,7 +232,13 @@ export default class Statement { } } - if ( node.type === 'Identifier' ) { + // we only care about writes that happen a) at the top level, + // or b) inside a function that could be immediately invoked. + // Writes inside named functions are only relevant if the + // function is called, in which case we don't need to do + // anything (but we still need to call checkForWrites to + // catch illegal reassignments to imported bindings) + if ( writeDepth === 0 && node.type === 'Identifier' ) { this.modifies[ node.name ] = true; } }; diff --git a/test/form/exclude-unnecessary-modifications/_config.js b/test/form/exclude-unnecessary-modifications/_config.js new file mode 100644 index 0000000..2b3de2b --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'statements that modify definitions within unused functions are excluded' +}; diff --git a/test/form/exclude-unnecessary-modifications/_expected/amd.js b/test/form/exclude-unnecessary-modifications/_expected/amd.js new file mode 100644 index 0000000..21da4e1 --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/_expected/amd.js @@ -0,0 +1,18 @@ +define(function () { 'use strict'; + + var foo = {}; + + mutate1( foo ); + + // should be included + [ 'a', 'b', 'c' ].forEach( function ( letter, i ) { + foo[ letter ] = i; + }) + + function mutate1 ( obj ) { + obj.mutated = 1; + } + + console.log( foo ); + +}); diff --git a/test/form/exclude-unnecessary-modifications/_expected/cjs.js b/test/form/exclude-unnecessary-modifications/_expected/cjs.js new file mode 100644 index 0000000..e3e9a0a --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/_expected/cjs.js @@ -0,0 +1,16 @@ +'use strict'; + +var foo = {}; + +mutate1( foo ); + +// should be included +[ 'a', 'b', 'c' ].forEach( function ( letter, i ) { + foo[ letter ] = i; +}) + +function mutate1 ( obj ) { + obj.mutated = 1; +} + +console.log( foo ); diff --git a/test/form/exclude-unnecessary-modifications/_expected/es6.js b/test/form/exclude-unnecessary-modifications/_expected/es6.js new file mode 100644 index 0000000..33e349f --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/_expected/es6.js @@ -0,0 +1,14 @@ +var foo = {}; + +mutate1( foo ); + +// should be included +[ 'a', 'b', 'c' ].forEach( function ( letter, i ) { + foo[ letter ] = i; +}) + +function mutate1 ( obj ) { + obj.mutated = 1; +} + +console.log( foo ); diff --git a/test/form/exclude-unnecessary-modifications/_expected/iife.js b/test/form/exclude-unnecessary-modifications/_expected/iife.js new file mode 100644 index 0000000..5919cbf --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/_expected/iife.js @@ -0,0 +1,18 @@ +(function () { 'use strict'; + + var foo = {}; + + mutate1( foo ); + + // should be included + [ 'a', 'b', 'c' ].forEach( function ( letter, i ) { + foo[ letter ] = i; + }) + + function mutate1 ( obj ) { + obj.mutated = 1; + } + + console.log( foo ); + +})(); diff --git a/test/form/exclude-unnecessary-modifications/_expected/umd.js b/test/form/exclude-unnecessary-modifications/_expected/umd.js new file mode 100644 index 0000000..58ecb8d --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/_expected/umd.js @@ -0,0 +1,22 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? factory() : + typeof define === 'function' && define.amd ? define(factory) : + factory(); +}(this, function () { 'use strict'; + + var foo = {}; + + mutate1( foo ); + + // should be included + [ 'a', 'b', 'c' ].forEach( function ( letter, i ) { + foo[ letter ] = i; + }) + + function mutate1 ( obj ) { + obj.mutated = 1; + } + + console.log( foo ); + +})); diff --git a/test/form/exclude-unnecessary-modifications/foo.js b/test/form/exclude-unnecessary-modifications/foo.js new file mode 100644 index 0000000..05f7e0b --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/foo.js @@ -0,0 +1,27 @@ +var foo = {}; + +mutate1( foo ); + +// should be included +[ 'a', 'b', 'c' ].forEach( function ( letter, i ) { + foo[ letter ] = i; +}) + +function mutate1 ( obj ) { + obj.mutated = 1; +} + +// should be excluded +var mutate2 = function () { + foo.mutated = 2; +} + +function unused1 () { + foo.wat = 'nope'; +} + +function unused2 () { + mutate1( foo ); +} + +export default foo; diff --git a/test/form/exclude-unnecessary-modifications/main.js b/test/form/exclude-unnecessary-modifications/main.js new file mode 100644 index 0000000..03d6edf --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/main.js @@ -0,0 +1,2 @@ +import foo from './foo'; +console.log( foo ); From cdc721cc71cc58f88c457f13f6d479a671f0b09c Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 19 Jul 2015 10:16:47 -0400 Subject: [PATCH 2/6] more tests for #37 --- .../_expected/amd.js | 6 ++++- .../_expected/cjs.js | 6 ++++- .../_expected/es6.js | 6 ++++- .../_expected/iife.js | 6 ++++- .../_expected/umd.js | 6 ++++- .../exclude-unnecessary-modifications/foo.js | 24 ++++++++++++++++++- 6 files changed, 48 insertions(+), 6 deletions(-) diff --git a/test/form/exclude-unnecessary-modifications/_expected/amd.js b/test/form/exclude-unnecessary-modifications/_expected/amd.js index 21da4e1..2cde378 100644 --- a/test/form/exclude-unnecessary-modifications/_expected/amd.js +++ b/test/form/exclude-unnecessary-modifications/_expected/amd.js @@ -7,7 +7,11 @@ define(function () { 'use strict'; // should be included [ 'a', 'b', 'c' ].forEach( function ( letter, i ) { foo[ letter ] = i; - }) + }); + + [ 'd', 'e', 'f' ].forEach( ( letter, i ) => { + foo[ letter ] = i; + }); function mutate1 ( obj ) { obj.mutated = 1; diff --git a/test/form/exclude-unnecessary-modifications/_expected/cjs.js b/test/form/exclude-unnecessary-modifications/_expected/cjs.js index e3e9a0a..c440046 100644 --- a/test/form/exclude-unnecessary-modifications/_expected/cjs.js +++ b/test/form/exclude-unnecessary-modifications/_expected/cjs.js @@ -7,7 +7,11 @@ mutate1( foo ); // should be included [ 'a', 'b', 'c' ].forEach( function ( letter, i ) { foo[ letter ] = i; -}) +}); + +[ 'd', 'e', 'f' ].forEach( ( letter, i ) => { + foo[ letter ] = i; +}); function mutate1 ( obj ) { obj.mutated = 1; diff --git a/test/form/exclude-unnecessary-modifications/_expected/es6.js b/test/form/exclude-unnecessary-modifications/_expected/es6.js index 33e349f..57365ac 100644 --- a/test/form/exclude-unnecessary-modifications/_expected/es6.js +++ b/test/form/exclude-unnecessary-modifications/_expected/es6.js @@ -5,7 +5,11 @@ mutate1( foo ); // should be included [ 'a', 'b', 'c' ].forEach( function ( letter, i ) { foo[ letter ] = i; -}) +}); + +[ 'd', 'e', 'f' ].forEach( ( letter, i ) => { + foo[ letter ] = i; +}); function mutate1 ( obj ) { obj.mutated = 1; diff --git a/test/form/exclude-unnecessary-modifications/_expected/iife.js b/test/form/exclude-unnecessary-modifications/_expected/iife.js index 5919cbf..3063464 100644 --- a/test/form/exclude-unnecessary-modifications/_expected/iife.js +++ b/test/form/exclude-unnecessary-modifications/_expected/iife.js @@ -7,7 +7,11 @@ // should be included [ 'a', 'b', 'c' ].forEach( function ( letter, i ) { foo[ letter ] = i; - }) + }); + + [ 'd', 'e', 'f' ].forEach( ( letter, i ) => { + foo[ letter ] = i; + }); function mutate1 ( obj ) { obj.mutated = 1; diff --git a/test/form/exclude-unnecessary-modifications/_expected/umd.js b/test/form/exclude-unnecessary-modifications/_expected/umd.js index 58ecb8d..55207a5 100644 --- a/test/form/exclude-unnecessary-modifications/_expected/umd.js +++ b/test/form/exclude-unnecessary-modifications/_expected/umd.js @@ -11,7 +11,11 @@ // should be included [ 'a', 'b', 'c' ].forEach( function ( letter, i ) { foo[ letter ] = i; - }) + }); + + [ 'd', 'e', 'f' ].forEach( ( letter, i ) => { + foo[ letter ] = i; + }); function mutate1 ( obj ) { obj.mutated = 1; diff --git a/test/form/exclude-unnecessary-modifications/foo.js b/test/form/exclude-unnecessary-modifications/foo.js index 05f7e0b..268c0b9 100644 --- a/test/form/exclude-unnecessary-modifications/foo.js +++ b/test/form/exclude-unnecessary-modifications/foo.js @@ -5,7 +5,11 @@ mutate1( foo ); // should be included [ 'a', 'b', 'c' ].forEach( function ( letter, i ) { foo[ letter ] = i; -}) +}); + +[ 'd', 'e', 'f' ].forEach( ( letter, i ) => { + foo[ letter ] = i; +}); function mutate1 ( obj ) { obj.mutated = 1; @@ -24,4 +28,22 @@ function unused2 () { mutate1( foo ); } +var obj1 = { + mutate3 () { foo.mutated = 3; } +}; + +var obj2 = { + mutate4: function () { foo.mutated = 4; } +}; + +var obj3 = { + mutate5: () => foo.mutated = 5 +}; + +class Mutator { + mutate6 () { + foo.mutated = 6; + } +} + export default foo; From 5e80ffa3aaa6f916e62764445b597ae6e155ff62 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Mon, 20 Jul 2015 09:53:29 -0400 Subject: [PATCH 3/6] another test for #37 --- test/form/exclude-unnecessary-modifications/_config.js | 3 ++- .../exclude-unnecessary-modifications/_expected/amd.js | 10 ++++++++-- .../exclude-unnecessary-modifications/_expected/cjs.js | 10 ++++++++-- .../exclude-unnecessary-modifications/_expected/es6.js | 10 ++++++++-- .../_expected/iife.js | 10 ++++++++-- .../exclude-unnecessary-modifications/_expected/umd.js | 10 ++++++++-- test/form/exclude-unnecessary-modifications/foo.js | 10 ++++++++-- 7 files changed, 50 insertions(+), 13 deletions(-) diff --git a/test/form/exclude-unnecessary-modifications/_config.js b/test/form/exclude-unnecessary-modifications/_config.js index 2b3de2b..c66ce85 100644 --- a/test/form/exclude-unnecessary-modifications/_config.js +++ b/test/form/exclude-unnecessary-modifications/_config.js @@ -1,3 +1,4 @@ module.exports = { - description: 'statements that modify definitions within unused functions are excluded' + description: 'statements that modify definitions within unused functions are excluded', + solo: true }; diff --git a/test/form/exclude-unnecessary-modifications/_expected/amd.js b/test/form/exclude-unnecessary-modifications/_expected/amd.js index 2cde378..84befa4 100644 --- a/test/form/exclude-unnecessary-modifications/_expected/amd.js +++ b/test/form/exclude-unnecessary-modifications/_expected/amd.js @@ -13,10 +13,16 @@ define(function () { 'use strict'; foo[ letter ] = i; }); - function mutate1 ( obj ) { - obj.mutated = 1; + function mutate1 () { + foo.mutated = 1; } + ({ + mutate2: function () { + foo.mutated = 2; + } + }).mutate2(); + console.log( foo ); }); diff --git a/test/form/exclude-unnecessary-modifications/_expected/cjs.js b/test/form/exclude-unnecessary-modifications/_expected/cjs.js index c440046..596c910 100644 --- a/test/form/exclude-unnecessary-modifications/_expected/cjs.js +++ b/test/form/exclude-unnecessary-modifications/_expected/cjs.js @@ -13,8 +13,14 @@ mutate1( foo ); foo[ letter ] = i; }); -function mutate1 ( obj ) { - obj.mutated = 1; +function mutate1 () { + foo.mutated = 1; } +({ + mutate2: function () { + foo.mutated = 2; + } +}).mutate2(); + console.log( foo ); diff --git a/test/form/exclude-unnecessary-modifications/_expected/es6.js b/test/form/exclude-unnecessary-modifications/_expected/es6.js index 57365ac..a88d21e 100644 --- a/test/form/exclude-unnecessary-modifications/_expected/es6.js +++ b/test/form/exclude-unnecessary-modifications/_expected/es6.js @@ -11,8 +11,14 @@ mutate1( foo ); foo[ letter ] = i; }); -function mutate1 ( obj ) { - obj.mutated = 1; +function mutate1 () { + foo.mutated = 1; } +({ + mutate2: function () { + foo.mutated = 2; + } +}).mutate2(); + console.log( foo ); diff --git a/test/form/exclude-unnecessary-modifications/_expected/iife.js b/test/form/exclude-unnecessary-modifications/_expected/iife.js index 3063464..0fd4c40 100644 --- a/test/form/exclude-unnecessary-modifications/_expected/iife.js +++ b/test/form/exclude-unnecessary-modifications/_expected/iife.js @@ -13,10 +13,16 @@ foo[ letter ] = i; }); - function mutate1 ( obj ) { - obj.mutated = 1; + function mutate1 () { + foo.mutated = 1; } + ({ + mutate2: function () { + foo.mutated = 2; + } + }).mutate2(); + console.log( foo ); })(); diff --git a/test/form/exclude-unnecessary-modifications/_expected/umd.js b/test/form/exclude-unnecessary-modifications/_expected/umd.js index 55207a5..1c2bb78 100644 --- a/test/form/exclude-unnecessary-modifications/_expected/umd.js +++ b/test/form/exclude-unnecessary-modifications/_expected/umd.js @@ -17,10 +17,16 @@ foo[ letter ] = i; }); - function mutate1 ( obj ) { - obj.mutated = 1; + function mutate1 () { + foo.mutated = 1; } + ({ + mutate2: function () { + foo.mutated = 2; + } + }).mutate2(); + console.log( foo ); })); diff --git a/test/form/exclude-unnecessary-modifications/foo.js b/test/form/exclude-unnecessary-modifications/foo.js index 268c0b9..71e1e93 100644 --- a/test/form/exclude-unnecessary-modifications/foo.js +++ b/test/form/exclude-unnecessary-modifications/foo.js @@ -11,10 +11,16 @@ mutate1( foo ); foo[ letter ] = i; }); -function mutate1 ( obj ) { - obj.mutated = 1; +function mutate1 () { + foo.mutated = 1; } +({ + mutate2: function () { + foo.mutated = 2; + } +}).mutate2(); + // should be excluded var mutate2 = function () { foo.mutated = 2; From 8a7a91c6435dbcd99f53e2ebb2e899ed9f2d0db1 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 21 Aug 2015 15:53:02 -0400 Subject: [PATCH 4/6] make test passable --- .../_config.js | 3 +-- .../exclude-unnecessary-modifications/foo.js | 18 ------------------ 2 files changed, 1 insertion(+), 20 deletions(-) diff --git a/test/form/exclude-unnecessary-modifications/_config.js b/test/form/exclude-unnecessary-modifications/_config.js index c66ce85..2b3de2b 100644 --- a/test/form/exclude-unnecessary-modifications/_config.js +++ b/test/form/exclude-unnecessary-modifications/_config.js @@ -1,4 +1,3 @@ module.exports = { - description: 'statements that modify definitions within unused functions are excluded', - solo: true + description: 'statements that modify definitions within unused functions are excluded' }; diff --git a/test/form/exclude-unnecessary-modifications/foo.js b/test/form/exclude-unnecessary-modifications/foo.js index 71e1e93..2280631 100644 --- a/test/form/exclude-unnecessary-modifications/foo.js +++ b/test/form/exclude-unnecessary-modifications/foo.js @@ -34,22 +34,4 @@ function unused2 () { mutate1( foo ); } -var obj1 = { - mutate3 () { foo.mutated = 3; } -}; - -var obj2 = { - mutate4: function () { foo.mutated = 4; } -}; - -var obj3 = { - mutate5: () => foo.mutated = 5 -}; - -class Mutator { - mutate6 () { - foo.mutated = 6; - } -} - export default foo; From e35df59450f85269810f47019e2c7f648e496478 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 21 Aug 2015 17:50:49 -0400 Subject: [PATCH 5/6] failing test for #92 --- test/form/exports-at-end-if-possible/_config.js | 7 +++++++ .../exports-at-end-if-possible/_expected/amd.js | 11 +++++++++++ .../exports-at-end-if-possible/_expected/cjs.js | 9 +++++++++ .../exports-at-end-if-possible/_expected/es6.js | 7 +++++++ .../exports-at-end-if-possible/_expected/iife.js | 11 +++++++++++ .../exports-at-end-if-possible/_expected/umd.js | 15 +++++++++++++++ test/form/exports-at-end-if-possible/main.js | 5 +++++ 7 files changed, 65 insertions(+) create mode 100644 test/form/exports-at-end-if-possible/_config.js create mode 100644 test/form/exports-at-end-if-possible/_expected/amd.js create mode 100644 test/form/exports-at-end-if-possible/_expected/cjs.js create mode 100644 test/form/exports-at-end-if-possible/_expected/es6.js create mode 100644 test/form/exports-at-end-if-possible/_expected/iife.js create mode 100644 test/form/exports-at-end-if-possible/_expected/umd.js create mode 100644 test/form/exports-at-end-if-possible/main.js 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..84caf3f --- /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..14fc9e9 --- /dev/null +++ b/test/form/exports-at-end-if-possible/_expected/es6.js @@ -0,0 +1,7 @@ +const 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/exports-at-end-if-possible/_expected/umd.js b/test/form/exports-at-end-if-possible/_expected/umd.js new file mode 100644 index 0000000..f30f365 --- /dev/null +++ b/test/form/exports-at-end-if-possible/_expected/umd.js @@ -0,0 +1,15 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) : + typeof define === 'function' && define.amd ? define(['exports'], factory) : + factory((global.myBundle = {})); +}(this, 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/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 ); From 71d9d4fa0eccbebe4a2ab50d17b563f20c4b42e9 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Fri, 21 Aug 2015 18:47:54 -0400 Subject: [PATCH 6/6] only rewrite exported vars as exports.foo if necessary --- src/Bundle.js | 8 +++--- src/Module.js | 25 +++++++++++++++---- src/Statement.js | 13 ++++++++++ test/form/exported-empty-vars/_config.js | 6 ----- .../form/exported-empty-vars/_expected/amd.js | 8 ------ .../form/exported-empty-vars/_expected/cjs.js | 6 ----- .../form/exported-empty-vars/_expected/es6.js | 9 ------- .../exported-empty-vars/_expected/iife.js | 8 ------ .../form/exported-empty-vars/_expected/umd.js | 12 --------- test/form/exported-empty-vars/bar.js | 6 ----- test/form/exported-empty-vars/foo.js | 2 -- test/form/exported-empty-vars/main.js | 2 -- .../exports-at-end-if-possible/_config.js | 2 +- .../_expected/es6.js | 2 +- test/form/multiple-exports/_expected/amd.js | 7 ++++-- test/form/multiple-exports/_expected/cjs.js | 7 ++++-- test/form/multiple-exports/_expected/iife.js | 7 ++++-- test/form/multiple-exports/_expected/umd.js | 7 ++++-- .../_expected/amd.js | 4 ++- .../_expected/cjs.js | 4 ++- .../_expected/iife.js | 4 ++- .../_expected/umd.js | 4 ++- .../function/assignment-to-exports/_config.js | 3 ++- 23 files changed, 73 insertions(+), 83 deletions(-) delete mode 100644 test/form/exported-empty-vars/_config.js delete mode 100644 test/form/exported-empty-vars/_expected/amd.js delete mode 100644 test/form/exported-empty-vars/_expected/cjs.js delete mode 100644 test/form/exported-empty-vars/_expected/es6.js delete mode 100644 test/form/exported-empty-vars/_expected/iife.js delete mode 100644 test/form/exported-empty-vars/_expected/umd.js delete mode 100644 test/form/exported-empty-vars/bar.js delete mode 100644 test/form/exported-empty-vars/foo.js delete mode 100644 test/form/exported-empty-vars/main.js diff --git a/src/Bundle.js b/src/Bundle.js index 0fd7656..aac7790 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 9964ae2..461c20b 100644 --- a/src/Module.js +++ b/src/Module.js @@ -64,7 +64,7 @@ export default class Module { this.replacements = blank(); - this.varDeclarations = []; + this.reassignments = []; this.definitions = blank(); this.definitionPromises = blank(); @@ -202,20 +202,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 b68d216..ad66b15 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -30,6 +30,8 @@ export default class Statement { this.dependsOn = blank(); this.stronglyDependsOn = blank(); + this.reassigns = blank(); + this.isIncluded = false; this.isImportDeclaration = node.type === 'ImportDeclaration'; @@ -227,6 +229,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, 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/_expected/umd.js b/test/form/exported-empty-vars/_expected/umd.js deleted file mode 100644 index 9cde4bd..0000000 --- a/test/form/exported-empty-vars/_expected/umd.js +++ /dev/null @@ -1,12 +0,0 @@ -(function (global, factory) { - typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) : - typeof define === 'function' && define.amd ? define(['exports'], factory) : - factory((global.myBundle = {})); -}(this, function (exports) { 'use strict'; - - exports.foo = 42; - - exports.bar = 43; - exports.baz = 44; - -})); 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 index 84caf3f..a898bc7 100644 --- a/test/form/exports-at-end-if-possible/_config.js +++ b/test/form/exports-at-end-if-possible/_config.js @@ -3,5 +3,5 @@ module.exports = { options: { moduleName: 'myBundle' }, - solo: true + // solo: true }; diff --git a/test/form/exports-at-end-if-possible/_expected/es6.js b/test/form/exports-at-end-if-possible/_expected/es6.js index 14fc9e9..b42b4e4 100644 --- a/test/form/exports-at-end-if-possible/_expected/es6.js +++ b/test/form/exports-at-end-if-possible/_expected/es6.js @@ -1,4 +1,4 @@ -const FOO = 'foo'; +var FOO = '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 };