From e0b690ad0f71ed62f388dc5745dd3860dfdcc2e6 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 15:41:19 -0400 Subject: [PATCH 01/29] first (failing) stab at better side-effect detection --- src/Declaration.js | 87 +++++++++++++++++++ src/Module.js | 4 + src/Statement.js | 52 ++++++++++- src/ast/Scope.js | 40 +-------- test/form/namespace-optimization-b/foo.js | 5 +- test/form/namespace-optimization-b/main.js | 17 ++-- .../rewrite-member-expressions/_config.js | 2 +- 7 files changed, 157 insertions(+), 50 deletions(-) create mode 100644 src/Declaration.js diff --git a/src/Declaration.js b/src/Declaration.js new file mode 100644 index 0000000..caefdca --- /dev/null +++ b/src/Declaration.js @@ -0,0 +1,87 @@ +import { walk } from 'estree-walker'; +import { keys } from './utils/object'; + +const modifierNodes = { + AssignmentExpression: 'left', + UpdateExpression: 'argument' +}; + +export default class Declaration { + constructor ( node ) { + if ( node ) { + if ( node.type === 'FunctionDeclaration' ) { + this.isFunctionDeclaration = true; + this.functionBody = node.body; + } else if ( node.type === 'VariableDeclarator' && node.init && /FunctionExpression/.test( node.init.type ) ) { + this.isFunctionDeclaration = true; + this.functionBody = node.init.body; + } + } + + this.statement = null; + this.name = null; + + this.isReassigned = false; + this.aliases = []; + } + + addAlias ( declaration ) { + this.aliases.push( declaration ); + } + + addReference ( reference ) { + reference.declaration = this; + this.name = reference.name; // TODO handle differences of opinion + + if ( reference.isReassignment ) this.isReassigned = true; + } + + mutates () { + // returns a list of things this function mutates when it gets called + if ( !this._mutates ) { + let mutatedNames = {}; + + const statement = this.statement; + let scope = statement.scope; + + const addNode = node => { + while ( node.type === 'MemberExpression' ) node = node.object; + if ( node.type === 'Identifier' ) mutatedNames[ node.name ] = true; + }; + + walk( this.functionBody, { + enter ( node ) { + if ( node._scope ) scope = node._scope; + + if ( node.type in modifierNodes ) { + addNode( node[ modifierNodes[ node.type ] ] ); + } else if ( node.type === 'CallExpression' ) { + addNode( node.callee ); + } + }, + + leave ( node ) { + if ( node._scope ) scope = scope.parent; + } + }); + + this._mutates = keys( mutatedNames ); + } + + return this._mutates; + } + + render ( es6 ) { + if ( es6 ) return this.name; + if ( !this.isReassigned || !this.isExported ) return this.name; + + return `exports.${this.name}`; + } + + use () { + this.isUsed = true; + if ( this.statement ) this.statement.mark(); + + this.aliases.forEach( alias => alias.use() ); + } +} diff --git a/src/Module.js b/src/Module.js index 3bbf32e..863a246 100644 --- a/src/Module.js +++ b/src/Module.js @@ -35,6 +35,10 @@ class SyntheticDefaultDeclaration { this.original = declaration; } + mutates () { + return this.original.mutates(); + } + render () { return !this.original || this.original.isReassigned ? this.name : diff --git a/src/Statement.js b/src/Statement.js index d5ec856..e0a83b1 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -185,15 +185,61 @@ export default class Statement { let hasSideEffect = false; walk( this.node, { - enter ( node, parent ) { - if ( /Function/.test( node.type ) && !isIife( node, parent ) ) return this.skip(); + enter ( node ) { + // Don't descend into (quasi) function declarations – we'll worry about those + // if they get called + if ( node.type === 'FunctionDeclaration' || node.type === 'VariableDeclarator' && node.init && /FunctionExpression/.test( node.init.type ) ) { + return this.skip(); + } // If this is a top-level call expression, or an assignment to a global, // this statement will need to be marked - if ( node.type === 'CallExpression' || node.type === 'NewExpression' ) { + if ( node.type === 'NewExpression' ) { hasSideEffect = true; } + else if ( node.type === 'CallExpression' ) { + if ( node.callee.type === 'Identifier' ) { + const declaration = statement.module.trace( node.callee.name ); + + if ( !declaration || !declaration.isFunctionDeclaration ) { + hasSideEffect = true; + } + + else { + const mutatedByFunction = declaration.mutates(); + let i = mutatedByFunction.length; + while ( i-- ) { + const mutatedDeclaration = statement.module.trace( mutatedByFunction[i] ); + + if ( !mutatedDeclaration || mutatedDeclaration.isUsed ) { + hasSideEffect = true; + break; + } + } + + // if ( declaration.hasSideEffect() ) { + // // if calling this function creates side-effects... + // hasSideEffect = true; + // } + // + // else { + // // ...or mutates inputs that are included... + // hasSideEffect = true; + // } + + // TODO does function mutate inputs that are needed? + } + } + + else if ( node.callee.type === 'MemberExpression' ) { + // if we're calling e.g. Object.keys(thing), there are no side-effects + // TODO + + hasSideEffect = true; + } + } + else if ( node.type in modifierNodes ) { let subject = node[ modifierNodes[ node.type ] ]; while ( subject.type === 'MemberExpression' ) subject = subject.object; diff --git a/src/ast/Scope.js b/src/ast/Scope.js index 7a210a4..a0d1350 100644 --- a/src/ast/Scope.js +++ b/src/ast/Scope.js @@ -1,4 +1,5 @@ import { blank, keys } from '../utils/object.js'; +import Declaration from '../Declaration.js'; const extractors = { Identifier ( names, param ) { @@ -33,41 +34,6 @@ function extractNames ( param ) { return names; } -class Declaration { - constructor () { - this.statement = null; - this.name = null; - - this.isReassigned = false; - this.aliases = []; - } - - addAlias ( declaration ) { - this.aliases.push( declaration ); - } - - addReference ( reference ) { - reference.declaration = this; - this.name = reference.name; // TODO handle differences of opinion - - if ( reference.isReassignment ) this.isReassigned = true; - } - - render ( es6 ) { - if ( es6 ) return this.name; - if ( !this.isReassigned || !this.isExported ) return this.name; - - return `exports.${this.name}`; - } - - use () { - this.isUsed = true; - if ( this.statement ) this.statement.mark(); - - this.aliases.forEach( alias => alias.use() ); - } -} - export default class Scope { constructor ( options ) { options = options || {}; @@ -80,7 +46,7 @@ export default class Scope { if ( options.params ) { options.params.forEach( param => { extractNames( param ).forEach( name => { - this.declarations[ name ] = new Declaration( name ); + this.declarations[ name ] = new Declaration(); }); }); } @@ -93,7 +59,7 @@ export default class Scope { this.parent.addDeclaration( node, isBlockDeclaration, isVar ); } else { extractNames( node.id ).forEach( name => { - this.declarations[ name ] = new Declaration( name ); + this.declarations[ name ] = new Declaration( node ); }); } } diff --git a/test/form/namespace-optimization-b/foo.js b/test/form/namespace-optimization-b/foo.js index 20630ba..05d87c4 100644 --- a/test/form/namespace-optimization-b/foo.js +++ b/test/form/namespace-optimization-b/foo.js @@ -1,2 +1,3 @@ -export function foo() { -}; +export function foo () { + console.log( 'foo' ); +} diff --git a/test/form/namespace-optimization-b/main.js b/test/form/namespace-optimization-b/main.js index 041ecb1..8671b36 100644 --- a/test/form/namespace-optimization-b/main.js +++ b/test/form/namespace-optimization-b/main.js @@ -1,10 +1,13 @@ import * as foo from './foo'; -function a() { - foo.foo(); - foo.foo(); - var a; - if (a.b) { - } +function a () { + foo.foo(); + foo.foo(); + + var a; + if ( a.b ) { + // empty + } } -a(); \ No newline at end of file + +a(); diff --git a/test/function/rewrite-member-expressions/_config.js b/test/function/rewrite-member-expressions/_config.js index 8fc81ea..ec5c856 100644 --- a/test/function/rewrite-member-expressions/_config.js +++ b/test/function/rewrite-member-expressions/_config.js @@ -1,3 +1,3 @@ module.exports = { description: 'rewrites identifiers at the head of member expressions' -}; \ No newline at end of file +}; From d3979bd6904bf902175382aac9d06f2c42dadc97 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 17:30:06 -0400 Subject: [PATCH 02/29] better side-effect detection --- src/Bundle.js | 11 +- src/Declaration.js | 213 ++++++++++++++---- src/ExternalModule.js | 39 +--- src/Module.js | 132 +---------- src/Statement.js | 107 ++------- src/ast/flatten.js | 16 ++ src/ast/modifierNodes.js | 4 + src/utils/testForSideEffects.js | 92 ++++++++ .../namespace-optimization-b/_expected/amd.js | 30 +-- .../namespace-optimization-b/_expected/cjs.js | 24 +- .../namespace-optimization-b/_expected/es6.js | 22 +- .../_expected/iife.js | 30 +-- .../namespace-optimization-b/_expected/umd.js | 32 +-- .../self-contained-bundle/_expected/amd.js | 2 +- .../self-contained-bundle/_expected/cjs.js | 2 +- .../self-contained-bundle/_expected/es6.js | 2 +- .../self-contained-bundle/_expected/iife.js | 2 +- .../self-contained-bundle/_expected/umd.js | 2 +- test/form/self-contained-bundle/foo.js | 2 +- test/form/side-effect-b/_config.js | 6 + test/form/side-effect-b/_expected/amd.js | 7 + test/form/side-effect-b/_expected/cjs.js | 5 + test/form/side-effect-b/_expected/es6.js | 3 + test/form/side-effect-b/_expected/iife.js | 7 + test/form/side-effect-b/_expected/umd.js | 11 + test/form/side-effect-b/main.js | 8 + test/form/side-effect-c/_config.js | 6 + test/form/side-effect-c/_expected/amd.js | 7 + test/form/side-effect-c/_expected/cjs.js | 5 + test/form/side-effect-c/_expected/es6.js | 3 + test/form/side-effect-c/_expected/iife.js | 7 + test/form/side-effect-c/_expected/umd.js | 11 + test/form/side-effect-c/main.js | 10 + test/form/side-effect-d/_config.js | 6 + test/form/side-effect-d/_expected/amd.js | 7 + test/form/side-effect-d/_expected/cjs.js | 5 + test/form/side-effect-d/_expected/es6.js | 3 + test/form/side-effect-d/_expected/iife.js | 7 + test/form/side-effect-d/_expected/umd.js | 11 + test/form/side-effect-d/main.js | 4 + .../_config.js | 0 .../_expected/amd.js | 0 .../_expected/cjs.js | 0 .../_expected/es6.js | 0 .../_expected/iife.js | 0 .../_expected/umd.js | 0 .../foo.js | 0 .../main.js | 0 .../_expected/amd.js | 2 + .../_expected/cjs.js | 2 + .../_expected/es6.js | 2 + .../_expected/iife.js | 2 + .../_expected/umd.js | 2 + .../main.js | 2 + 54 files changed, 545 insertions(+), 370 deletions(-) create mode 100644 src/ast/flatten.js create mode 100644 src/ast/modifierNodes.js create mode 100644 src/utils/testForSideEffects.js create mode 100644 test/form/side-effect-b/_config.js create mode 100644 test/form/side-effect-b/_expected/amd.js create mode 100644 test/form/side-effect-b/_expected/cjs.js create mode 100644 test/form/side-effect-b/_expected/es6.js create mode 100644 test/form/side-effect-b/_expected/iife.js create mode 100644 test/form/side-effect-b/_expected/umd.js create mode 100644 test/form/side-effect-b/main.js create mode 100644 test/form/side-effect-c/_config.js create mode 100644 test/form/side-effect-c/_expected/amd.js create mode 100644 test/form/side-effect-c/_expected/cjs.js create mode 100644 test/form/side-effect-c/_expected/es6.js create mode 100644 test/form/side-effect-c/_expected/iife.js create mode 100644 test/form/side-effect-c/_expected/umd.js create mode 100644 test/form/side-effect-c/main.js create mode 100644 test/form/side-effect-d/_config.js create mode 100644 test/form/side-effect-d/_expected/amd.js create mode 100644 test/form/side-effect-d/_expected/cjs.js create mode 100644 test/form/side-effect-d/_expected/es6.js create mode 100644 test/form/side-effect-d/_expected/iife.js create mode 100644 test/form/side-effect-d/_expected/umd.js create mode 100644 test/form/side-effect-d/main.js rename test/form/{unused-side-effect => side-effect}/_config.js (100%) rename test/form/{unused-side-effect => side-effect}/_expected/amd.js (100%) rename test/form/{unused-side-effect => side-effect}/_expected/cjs.js (100%) rename test/form/{unused-side-effect => side-effect}/_expected/es6.js (100%) rename test/form/{unused-side-effect => side-effect}/_expected/iife.js (100%) rename test/form/{unused-side-effect => side-effect}/_expected/umd.js (100%) rename test/form/{unused-side-effect => side-effect}/foo.js (100%) rename test/form/{unused-side-effect => side-effect}/main.js (100%) diff --git a/src/Bundle.js b/src/Bundle.js index c4c5edf..8bc5826 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -73,14 +73,9 @@ export default class Bundle { declaration.use(); }); - let settled = false; - while ( !settled ) { - settled = true; - - this.modules.forEach( module => { - if ( module.markAllSideEffects() ) settled = false; - }); - } + this.modules.forEach( module => { + module.markAllSideEffects(); + }); this.orderedModules = this.sort(); this.deconflict(); diff --git a/src/Declaration.js b/src/Declaration.js index caefdca..7fb7c86 100644 --- a/src/Declaration.js +++ b/src/Declaration.js @@ -1,10 +1,5 @@ -import { walk } from 'estree-walker'; -import { keys } from './utils/object'; - -const modifierNodes = { - AssignmentExpression: 'left', - UpdateExpression: 'argument' -}; +import { blank, keys } from './utils/object.js'; +import testForSideEffects from './utils/testForSideEffects.js'; export default class Declaration { constructor ( node ) { @@ -22,6 +17,7 @@ export default class Declaration { this.name = null; this.isReassigned = false; + this.mutations = []; this.aliases = []; } @@ -34,54 +30,195 @@ export default class Declaration { this.name = reference.name; // TODO handle differences of opinion if ( reference.isReassignment ) this.isReassigned = true; + if ( reference.isMutation && !~this.mutations.indexOf( reference.statement ) ) { + this.mutations.push( reference.statement ); + } } - mutates () { - // returns a list of things this function mutates when it gets called - if ( !this._mutates ) { - let mutatedNames = {}; + hasSideEffect () { + return testForSideEffects( this.functionBody, this.statement.scope, this.statement ); + } - const statement = this.statement; - let scope = statement.scope; + render ( es6 ) { + if ( es6 ) return this.name; + if ( !this.isReassigned || !this.isExported ) return this.name; - const addNode = node => { - while ( node.type === 'MemberExpression' ) node = node.object; - if ( node.type === 'Identifier' ) mutatedNames[ node.name ] = true; - }; + return `exports.${this.name}`; + } - walk( this.functionBody, { - enter ( node ) { - if ( node._scope ) scope = node._scope; + use () { + this.isUsed = true; + if ( this.statement ) this.statement.mark(); - if ( node.type in modifierNodes ) { - addNode( node[ modifierNodes[ node.type ] ] ); - } else if ( node.type === 'CallExpression' ) { - addNode( node.callee ); - } - }, + this.aliases.forEach( alias => alias.use() ); + } +} - leave ( node ) { - if ( node._scope ) scope = scope.parent; - } - }); +export class SyntheticDefaultDeclaration { + constructor ( node, statement, name ) { + this.node = node; + this.statement = statement; + this.name = name; - this._mutates = keys( mutatedNames ); - } + this.original = null; + this.isExported = false; + this.aliases = []; + } - return this._mutates; + addAlias ( declaration ) { + this.aliases.push( declaration ); } - render ( es6 ) { - if ( es6 ) return this.name; - if ( !this.isReassigned || !this.isExported ) return this.name; + addReference ( reference ) { + // Don't change the name to `default`; it's not a valid identifier name. + if ( reference.name === 'default' ) return; - return `exports.${this.name}`; + reference.declaration = this; + this.name = reference.name; + } + + bind ( declaration ) { + this.original = declaration; + } + + hasSideEffect () { + if ( this.original ) { + return this.original.hasSideEffect(); + } + + if ( /FunctionExpression/.test( this.node.declaration.type ) ) { + return testForSideEffects( this.node.declaration.body, this.statement.scope, this.statement ); + } + } + + render () { + return !this.original || this.original.isReassigned ? + this.name : + this.original.render(); } use () { this.isUsed = true; - if ( this.statement ) this.statement.mark(); + this.statement.mark(); + + if ( this.original ) this.original.use(); this.aliases.forEach( alias => alias.use() ); } } + +export class SyntheticNamespaceDeclaration { + constructor ( module ) { + this.module = module; + this.name = null; + + this.needsNamespaceBlock = false; + this.aliases = []; + + this.originals = blank(); + module.getExports().forEach( name => { + this.originals[ name ] = module.traceExport( name ); + }); + } + + addAlias ( declaration ) { + this.aliases.push( declaration ); + } + + addReference ( reference ) { + // if we have e.g. `foo.bar`, we can optimise + // the reference by pointing directly to `bar` + if ( reference.parts.length ) { + reference.name = reference.parts.shift(); + + reference.end += reference.name.length + 1; // TODO this is brittle + + const original = this.originals[ reference.name ]; + + // throw with an informative error message if the reference doesn't exist. + if ( !original ) { + this.module.bundle.onwarn( `Export '${reference.name}' is not defined by '${this.module.id}'` ); + reference.isUndefined = true; + return; + } + + original.addReference( reference ); + return; + } + + // otherwise we're accessing the namespace directly, + // which means we need to mark all of this module's + // exports and render a namespace block in the bundle + if ( !this.needsNamespaceBlock ) { + this.needsNamespaceBlock = true; + this.module.bundle.internalNamespaces.push( this ); + } + + reference.declaration = this; + this.name = reference.name; + } + + renderBlock ( indentString ) { + const members = keys( this.originals ).map( name => { + const original = this.originals[ name ]; + + if ( original.isReassigned ) { + return `${indentString}get ${name} () { return ${original.render()}; }`; + } + + return `${indentString}${name}: ${original.render()}`; + }); + + return `var ${this.render()} = Object.freeze({\n${members.join( ',\n' )}\n});\n\n`; + } + + render () { + return this.name; + } + + use () { + keys( this.originals ).forEach( name => { + this.originals[ name ].use(); + }); + + this.aliases.forEach( alias => alias.use() ); + } +} + +export class ExternalDeclaration { + constructor ( module, name ) { + this.module = module; + this.name = name; + this.isExternal = true; + } + + addAlias () { + // noop + } + + addReference ( reference ) { + reference.declaration = this; + + if ( this.name === 'default' || this.name === '*' ) { + this.module.suggestName( reference.name ); + } + } + + render ( es6 ) { + if ( this.name === '*' ) { + return this.module.name; + } + + if ( this.name === 'default' ) { + return !es6 && this.module.exportsNames ? + `${this.module.name}__default` : + this.module.name; + } + + return es6 ? this.name : `${this.module.name}.${this.name}`; + } + + use () { + // noop? + } +} diff --git a/src/ExternalModule.js b/src/ExternalModule.js index 0e6a3ac..3113a5d 100644 --- a/src/ExternalModule.js +++ b/src/ExternalModule.js @@ -1,43 +1,6 @@ import { blank } from './utils/object.js'; import makeLegalIdentifier from './utils/makeLegalIdentifier.js'; - -class ExternalDeclaration { - constructor ( module, name ) { - this.module = module; - this.name = name; - this.isExternal = true; - } - - addAlias () { - // noop - } - - addReference ( reference ) { - reference.declaration = this; - - if ( this.name === 'default' || this.name === '*' ) { - this.module.suggestName( reference.name ); - } - } - - render ( es6 ) { - if ( this.name === '*' ) { - return this.module.name; - } - - if ( this.name === 'default' ) { - return !es6 && this.module.exportsNames ? - `${this.module.name}__default` : - this.module.name; - } - - return es6 ? this.name : `${this.module.name}.${this.name}`; - } - - use () { - // noop? - } -} +import { ExternalDeclaration } from './Declaration.js'; export default class ExternalModule { constructor ( id ) { diff --git a/src/Module.js b/src/Module.js index 863a246..1e5a97b 100644 --- a/src/Module.js +++ b/src/Module.js @@ -7,131 +7,7 @@ import { basename, extname } from './utils/path.js'; import getLocation from './utils/getLocation.js'; import makeLegalIdentifier from './utils/makeLegalIdentifier.js'; import SOURCEMAPPING_URL from './utils/sourceMappingURL.js'; - -class SyntheticDefaultDeclaration { - constructor ( node, statement, name ) { - this.node = node; - this.statement = statement; - this.name = name; - - this.original = null; - this.isExported = false; - this.aliases = []; - } - - addAlias ( declaration ) { - this.aliases.push( declaration ); - } - - addReference ( reference ) { - // Don't change the name to `default`; it's not a valid identifier name. - if ( reference.name === 'default' ) return; - - reference.declaration = this; - this.name = reference.name; - } - - bind ( declaration ) { - this.original = declaration; - } - - mutates () { - return this.original.mutates(); - } - - render () { - return !this.original || this.original.isReassigned ? - this.name : - this.original.render(); - } - - use () { - this.isUsed = true; - this.statement.mark(); - - if ( this.original ) this.original.use(); - - this.aliases.forEach( alias => alias.use() ); - } -} - -class SyntheticNamespaceDeclaration { - constructor ( module ) { - this.module = module; - this.name = null; - - this.needsNamespaceBlock = false; - this.aliases = []; - - this.originals = blank(); - module.getExports().forEach( name => { - this.originals[ name ] = module.traceExport( name ); - }); - } - - addAlias ( declaration ) { - this.aliases.push( declaration ); - } - - addReference ( reference ) { - // if we have e.g. `foo.bar`, we can optimise - // the reference by pointing directly to `bar` - if ( reference.parts.length ) { - reference.name = reference.parts.shift(); - - reference.end += reference.name.length + 1; // TODO this is brittle - - const original = this.originals[ reference.name ]; - - // throw with an informative error message if the reference doesn't exist. - if ( !original ) { - this.module.bundle.onwarn( `Export '${reference.name}' is not defined by '${this.module.id}'` ); - reference.isUndefined = true; - return; - } - - original.addReference( reference ); - return; - } - - // otherwise we're accessing the namespace directly, - // which means we need to mark all of this module's - // exports and render a namespace block in the bundle - if ( !this.needsNamespaceBlock ) { - this.needsNamespaceBlock = true; - this.module.bundle.internalNamespaces.push( this ); - } - - reference.declaration = this; - this.name = reference.name; - } - - renderBlock ( indentString ) { - const members = keys( this.originals ).map( name => { - const original = this.originals[ name ]; - - if ( original.isReassigned ) { - return `${indentString}get ${name} () { return ${original.render()}; }`; - } - - return `${indentString}${name}: ${original.render()}`; - }); - - return `var ${this.render()} = Object.freeze({\n${members.join( ',\n' )}\n});\n\n`; - } - - render () { - return this.name; - } - - use () { - keys( this.originals ).forEach( name => { - this.originals[ name ].use(); - }); - - this.aliases.forEach( alias => alias.use() ); - } -} +import { SyntheticDefaultDeclaration, SyntheticNamespaceDeclaration } from './Declaration.js'; export default class Module { constructor ({ id, code, originalCode, ast, sourceMapChain, bundle }) { @@ -411,13 +287,9 @@ export default class Module { } markAllSideEffects () { - let hasSideEffect = false; - this.statements.forEach( statement => { - if ( statement.markSideEffect() ) hasSideEffect = true; + statement.markSideEffect(); }); - - return hasSideEffect; } namespace () { diff --git a/src/Statement.js b/src/Statement.js index e0a83b1..31b8bd7 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -1,16 +1,9 @@ import { walk } from 'estree-walker'; import Scope from './ast/Scope.js'; import attachScopes from './ast/attachScopes.js'; +import modifierNodes from './ast/modifierNodes.js'; import getLocation from './utils/getLocation.js'; - -const modifierNodes = { - AssignmentExpression: 'left', - UpdateExpression: 'argument' -}; - -function isIife ( node, parent ) { - return parent && parent.type === 'CallExpression' && node === parent.callee; -} +import testForSideEffects from './utils/testForSideEffects.js'; function isReference ( node, parent ) { if ( node.type === 'MemberExpression' ) { @@ -35,9 +28,10 @@ function isReference ( node, parent ) { } class Reference { - constructor ( node, scope ) { + constructor ( node, scope, statement ) { this.node = node; this.scope = scope; + this.statement = statement; this.declaration = null; // bound later @@ -90,6 +84,7 @@ export default class Statement { }); // find references + const statement = this; let { module, references, scope, stringLiteralRanges } = this; let readDepth = 0; @@ -106,7 +101,7 @@ export default class Statement { } if ( node._scope ) scope = node._scope; - if ( /Function/.test( node.type ) && !isIife( node, parent ) ) readDepth += 1; + if ( /Function/.test( node.type ) ) readDepth += 1; // special case – shorthand properties. because node.key === node.value, // we can't differentiate once we've descended into the node @@ -117,9 +112,10 @@ export default class Statement { return this.skip(); } + const isMutation = parent && parent.type in modifierNodes; let isReassignment; - if ( parent && parent.type in modifierNodes ) { + if ( isMutation ) { let subject = parent[ modifierNodes[ parent.type ] ]; let depth = 0; @@ -153,18 +149,19 @@ export default class Statement { scope.parent : scope; - const reference = new Reference( node, referenceScope ); + const reference = new Reference( node, referenceScope, statement ); references.push( reference ); reference.isImmediatelyUsed = !readDepth; reference.isReassignment = isReassignment; + reference.isMutation = !readDepth && isMutation; this.skip(); // don't descend from `foo.bar.baz` into `foo.bar` } }, - leave ( node, parent ) { + leave ( node ) { if ( node._scope ) scope = scope.parent; - if ( /Function/.test( node.type ) && !isIife( node, parent ) ) readDepth -= 1; + if ( /Function/.test( node.type ) ) readDepth -= 1; } }); } @@ -181,82 +178,10 @@ export default class Statement { markSideEffect () { if ( this.isIncluded ) return; - const statement = this; - let hasSideEffect = false; - - walk( this.node, { - enter ( node ) { - // Don't descend into (quasi) function declarations – we'll worry about those - // if they get called - if ( node.type === 'FunctionDeclaration' || node.type === 'VariableDeclarator' && node.init && /FunctionExpression/.test( node.init.type ) ) { - return this.skip(); - } - - // If this is a top-level call expression, or an assignment to a global, - // this statement will need to be marked - if ( node.type === 'NewExpression' ) { - hasSideEffect = true; - } - - else if ( node.type === 'CallExpression' ) { - if ( node.callee.type === 'Identifier' ) { - const declaration = statement.module.trace( node.callee.name ); - - if ( !declaration || !declaration.isFunctionDeclaration ) { - hasSideEffect = true; - } - - else { - const mutatedByFunction = declaration.mutates(); - let i = mutatedByFunction.length; - while ( i-- ) { - const mutatedDeclaration = statement.module.trace( mutatedByFunction[i] ); - - if ( !mutatedDeclaration || mutatedDeclaration.isUsed ) { - hasSideEffect = true; - break; - } - } - - // if ( declaration.hasSideEffect() ) { - // // if calling this function creates side-effects... - // hasSideEffect = true; - // } - // - // else { - // // ...or mutates inputs that are included... - // hasSideEffect = true; - // } - - // TODO does function mutate inputs that are needed? - } - } - - else if ( node.callee.type === 'MemberExpression' ) { - // if we're calling e.g. Object.keys(thing), there are no side-effects - // TODO - - hasSideEffect = true; - } - } - - else if ( node.type in modifierNodes ) { - let subject = node[ modifierNodes[ node.type ] ]; - while ( subject.type === 'MemberExpression' ) subject = subject.object; - - const declaration = statement.module.trace( subject.name ); - - if ( !declaration || declaration.isExternal || declaration.statement.isIncluded ) { - hasSideEffect = true; - } - } - - if ( hasSideEffect ) this.skip(); - } - }); - - if ( hasSideEffect ) statement.mark(); - return hasSideEffect; + if ( testForSideEffects( this.node, this.scope, this ) ) { + this.mark(); + return true; + } } source () { diff --git a/src/ast/flatten.js b/src/ast/flatten.js new file mode 100644 index 0000000..0cecbde --- /dev/null +++ b/src/ast/flatten.js @@ -0,0 +1,16 @@ +export default function flatten ( node ) { + let parts = []; + while ( node.type === 'MemberExpression' ) { + if ( node.computed ) return null; + parts.unshift( node.property.name ); + + node = node.object; + } + + if ( node.type !== 'Identifier' ) return null; + + const name = node.name; + parts.unshift( name ); + + return { name, keypath: parts.join( '.' ) }; +} diff --git a/src/ast/modifierNodes.js b/src/ast/modifierNodes.js new file mode 100644 index 0000000..2ea3f5d --- /dev/null +++ b/src/ast/modifierNodes.js @@ -0,0 +1,4 @@ +export default { + AssignmentExpression: 'left', + UpdateExpression: 'argument' +}; diff --git a/src/utils/testForSideEffects.js b/src/utils/testForSideEffects.js new file mode 100644 index 0000000..1aa4f96 --- /dev/null +++ b/src/utils/testForSideEffects.js @@ -0,0 +1,92 @@ +import { walk } from 'estree-walker'; +import modifierNodes from '../ast/modifierNodes.js'; +import flatten from '../ast/flatten'; + +let pureFunctions = {}; +[ + // TODO add others to this list + 'Object.keys' +].forEach( name => pureFunctions[ name ] = true ); + +export default function testForSideEffects ( node, scope, statement ) { + let hasSideEffect = false; + + walk( node, { + enter ( node ) { + if ( hasSideEffect ) return this.skip(); + if ( /Function/.test( node.type ) ) return this.skip(); + + if ( node._scope ) scope = node._scope; + + // If this is a top-level call expression, or an assignment to a global, + // this statement will need to be marked + if ( node.type === 'NewExpression' ) { + hasSideEffect = true; + return this.skip(); + } + + if ( node.type === 'CallExpression' ) { + if ( node.callee.type === 'Identifier' && !scope.contains( node.callee.name ) ) { + const declaration = statement.module.trace( node.callee.name ); + + if ( !declaration || declaration.isExternal ) { + // we're calling a global or an external function. Assume side-effects + hasSideEffect = true; + return this.skip(); + } + + // we're calling a function defined in this bundle + if ( declaration.hasSideEffect() ) { + hasSideEffect = true; + return this.skip(); + } + + // TODO does function mutate inputs that are needed? + return; + } + + if ( node.callee.type === 'MemberExpression' ) { + const flattened = flatten( node.callee ); + + if ( !flattened ) { + // is not a keypath like `foo.bar.baz` – could be e.g. + // `(a || b).foo()`. Err on the side of caution + hasSideEffect = true; + return; + } + + // if we're calling e.g. Object.keys(thing), there are no side-effects + // TODO make pureFunctions configurable + const declaration = statement.module.trace( flattened.name ); + if ( !declaration && pureFunctions[ flattened.keypath ] ) return; + + hasSideEffect = true; + return this.skip(); + } + + // otherwise we're probably dealing with a function expression + if ( testForSideEffects( node.callee, scope, statement ) ) { + hasSideEffect = true; + return this.skip(); + } + } + + if ( node.type in modifierNodes ) { + let subject = node[ modifierNodes[ node.type ] ]; + while ( subject.type === 'MemberExpression' ) subject = subject.object; + + const declaration = statement.module.trace( subject.name ); + + if ( !declaration || declaration.isExternal || declaration.statement.isIncluded ) { + hasSideEffect = true; + return this.skip(); + } + } + }, + leave ( node ) { + if ( node._scope ) scope = scope.parent; + } + }); + + return hasSideEffect; +} diff --git a/test/form/namespace-optimization-b/_expected/amd.js b/test/form/namespace-optimization-b/_expected/amd.js index 44fba59..30c5406 100644 --- a/test/form/namespace-optimization-b/_expected/amd.js +++ b/test/form/namespace-optimization-b/_expected/amd.js @@ -1,15 +1,19 @@ define(function () { 'use strict'; - function foo() { - }; - - function a() { - foo(); - foo(); - var a; - if (a.b) { - } - } - a(); - -}); + function foo () { + console.log( 'foo' ); + } + + function a () { + foo(); + foo(); + + var a; + if ( a.b ) { + // empty + } + } + + a(); + +}); \ No newline at end of file diff --git a/test/form/namespace-optimization-b/_expected/cjs.js b/test/form/namespace-optimization-b/_expected/cjs.js index 34f819f..3ee6869 100644 --- a/test/form/namespace-optimization-b/_expected/cjs.js +++ b/test/form/namespace-optimization-b/_expected/cjs.js @@ -1,13 +1,17 @@ 'use strict'; -function foo() { -}; - -function a() { - foo(); - foo(); - var a; - if (a.b) { - } +function foo () { + console.log( 'foo' ); } -a(); + +function a () { + foo(); + foo(); + + var a; + if ( a.b ) { + // empty + } +} + +a(); \ No newline at end of file diff --git a/test/form/namespace-optimization-b/_expected/es6.js b/test/form/namespace-optimization-b/_expected/es6.js index c1a6d5d..8346d0d 100644 --- a/test/form/namespace-optimization-b/_expected/es6.js +++ b/test/form/namespace-optimization-b/_expected/es6.js @@ -1,11 +1,15 @@ -function foo() { -}; +function foo () { + console.log( 'foo' ); +} + +function a () { + foo(); + foo(); -function a() { - foo(); - foo(); - var a; - if (a.b) { - } + var a; + if ( a.b ) { + // empty + } } -a(); + +a(); \ No newline at end of file diff --git a/test/form/namespace-optimization-b/_expected/iife.js b/test/form/namespace-optimization-b/_expected/iife.js index 6ea6190..59b2ce7 100644 --- a/test/form/namespace-optimization-b/_expected/iife.js +++ b/test/form/namespace-optimization-b/_expected/iife.js @@ -1,15 +1,19 @@ (function () { 'use strict'; - function foo() { - }; - - function a() { - foo(); - foo(); - var a; - if (a.b) { - } - } - a(); - -})(); + function foo () { + console.log( 'foo' ); + } + + function a () { + foo(); + foo(); + + var a; + if ( a.b ) { + // empty + } + } + + a(); + +})(); \ No newline at end of file diff --git a/test/form/namespace-optimization-b/_expected/umd.js b/test/form/namespace-optimization-b/_expected/umd.js index 108f27b..c2ad84b 100644 --- a/test/form/namespace-optimization-b/_expected/umd.js +++ b/test/form/namespace-optimization-b/_expected/umd.js @@ -1,19 +1,23 @@ (function (global, factory) { - typeof exports === 'object' && typeof module !== 'undefined' ? factory() : - typeof define === 'function' && define.amd ? define(factory) : - factory(); + typeof exports === 'object' && typeof module !== 'undefined' ? factory() : + typeof define === 'function' && define.amd ? define(factory) : + factory(); }(this, function () { 'use strict'; - function foo() { - }; + function foo () { + console.log( 'foo' ); + } - function a() { - foo(); - foo(); - var a; - if (a.b) { - } - } - a(); + function a () { + foo(); + foo(); -})); + var a; + if ( a.b ) { + // empty + } + } + + a(); + +})); \ No newline at end of file diff --git a/test/form/self-contained-bundle/_expected/amd.js b/test/form/self-contained-bundle/_expected/amd.js index ed04c7c..a29d399 100644 --- a/test/form/self-contained-bundle/_expected/amd.js +++ b/test/form/self-contained-bundle/_expected/amd.js @@ -1,7 +1,7 @@ define(function () { 'use strict'; function foo () { - return bar(); + console.log( bar() ); } function bar () { diff --git a/test/form/self-contained-bundle/_expected/cjs.js b/test/form/self-contained-bundle/_expected/cjs.js index 8a71fe7..499be1d 100644 --- a/test/form/self-contained-bundle/_expected/cjs.js +++ b/test/form/self-contained-bundle/_expected/cjs.js @@ -1,7 +1,7 @@ 'use strict'; function foo () { - return bar(); + console.log( bar() ); } function bar () { diff --git a/test/form/self-contained-bundle/_expected/es6.js b/test/form/self-contained-bundle/_expected/es6.js index 7d5460c..5588fc7 100644 --- a/test/form/self-contained-bundle/_expected/es6.js +++ b/test/form/self-contained-bundle/_expected/es6.js @@ -1,5 +1,5 @@ function foo () { - return bar(); + console.log( bar() ); } function bar () { diff --git a/test/form/self-contained-bundle/_expected/iife.js b/test/form/self-contained-bundle/_expected/iife.js index 46f6561..1017054 100644 --- a/test/form/self-contained-bundle/_expected/iife.js +++ b/test/form/self-contained-bundle/_expected/iife.js @@ -1,7 +1,7 @@ (function () { 'use strict'; function foo () { - return bar(); + console.log( bar() ); } function bar () { diff --git a/test/form/self-contained-bundle/_expected/umd.js b/test/form/self-contained-bundle/_expected/umd.js index 51b72b0..d29ef88 100644 --- a/test/form/self-contained-bundle/_expected/umd.js +++ b/test/form/self-contained-bundle/_expected/umd.js @@ -5,7 +5,7 @@ }(this, function () { 'use strict'; function foo () { - return bar(); + console.log( bar() ); } function bar () { diff --git a/test/form/self-contained-bundle/foo.js b/test/form/self-contained-bundle/foo.js index 1e911f3..46a9052 100644 --- a/test/form/self-contained-bundle/foo.js +++ b/test/form/self-contained-bundle/foo.js @@ -1,5 +1,5 @@ export default function foo () { - return bar(); + console.log( bar() ); } function bar () { diff --git a/test/form/side-effect-b/_config.js b/test/form/side-effect-b/_config.js new file mode 100644 index 0000000..9b0c852 --- /dev/null +++ b/test/form/side-effect-b/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: 'discards IIFE with no side-effects', + options: { + moduleName: 'myBundle' + } +}; diff --git a/test/form/side-effect-b/_expected/amd.js b/test/form/side-effect-b/_expected/amd.js new file mode 100644 index 0000000..70cc3c4 --- /dev/null +++ b/test/form/side-effect-b/_expected/amd.js @@ -0,0 +1,7 @@ +define(function () { 'use strict'; + + var main = 42; + + return main; + +}); \ No newline at end of file diff --git a/test/form/side-effect-b/_expected/cjs.js b/test/form/side-effect-b/_expected/cjs.js new file mode 100644 index 0000000..cc51dcb --- /dev/null +++ b/test/form/side-effect-b/_expected/cjs.js @@ -0,0 +1,5 @@ +'use strict'; + +var main = 42; + +module.exports = main; \ No newline at end of file diff --git a/test/form/side-effect-b/_expected/es6.js b/test/form/side-effect-b/_expected/es6.js new file mode 100644 index 0000000..b953a15 --- /dev/null +++ b/test/form/side-effect-b/_expected/es6.js @@ -0,0 +1,3 @@ +var main = 42; + +export default main; \ No newline at end of file diff --git a/test/form/side-effect-b/_expected/iife.js b/test/form/side-effect-b/_expected/iife.js new file mode 100644 index 0000000..0880329 --- /dev/null +++ b/test/form/side-effect-b/_expected/iife.js @@ -0,0 +1,7 @@ +var myBundle = (function () { 'use strict'; + + var main = 42; + + return main; + +})(); \ No newline at end of file diff --git a/test/form/side-effect-b/_expected/umd.js b/test/form/side-effect-b/_expected/umd.js new file mode 100644 index 0000000..7f83fb9 --- /dev/null +++ b/test/form/side-effect-b/_expected/umd.js @@ -0,0 +1,11 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() : + typeof define === 'function' && define.amd ? define(factory) : + global.myBundle = factory(); +}(this, function () { 'use strict'; + + var main = 42; + + return main; + +})); \ No newline at end of file diff --git a/test/form/side-effect-b/main.js b/test/form/side-effect-b/main.js new file mode 100644 index 0000000..c5e65eb --- /dev/null +++ b/test/form/side-effect-b/main.js @@ -0,0 +1,8 @@ +var Unused = (function () { + function Unused () {} + Unused.prototype = {}; + + return Unused; +}()); + +export default 42; diff --git a/test/form/side-effect-c/_config.js b/test/form/side-effect-c/_config.js new file mode 100644 index 0000000..0d692d4 --- /dev/null +++ b/test/form/side-effect-c/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: 'discards function with no side-effects', + options: { + moduleName: 'myBundle' + } +}; diff --git a/test/form/side-effect-c/_expected/amd.js b/test/form/side-effect-c/_expected/amd.js new file mode 100644 index 0000000..70cc3c4 --- /dev/null +++ b/test/form/side-effect-c/_expected/amd.js @@ -0,0 +1,7 @@ +define(function () { 'use strict'; + + var main = 42; + + return main; + +}); \ No newline at end of file diff --git a/test/form/side-effect-c/_expected/cjs.js b/test/form/side-effect-c/_expected/cjs.js new file mode 100644 index 0000000..cc51dcb --- /dev/null +++ b/test/form/side-effect-c/_expected/cjs.js @@ -0,0 +1,5 @@ +'use strict'; + +var main = 42; + +module.exports = main; \ No newline at end of file diff --git a/test/form/side-effect-c/_expected/es6.js b/test/form/side-effect-c/_expected/es6.js new file mode 100644 index 0000000..b953a15 --- /dev/null +++ b/test/form/side-effect-c/_expected/es6.js @@ -0,0 +1,3 @@ +var main = 42; + +export default main; \ No newline at end of file diff --git a/test/form/side-effect-c/_expected/iife.js b/test/form/side-effect-c/_expected/iife.js new file mode 100644 index 0000000..0880329 --- /dev/null +++ b/test/form/side-effect-c/_expected/iife.js @@ -0,0 +1,7 @@ +var myBundle = (function () { 'use strict'; + + var main = 42; + + return main; + +})(); \ No newline at end of file diff --git a/test/form/side-effect-c/_expected/umd.js b/test/form/side-effect-c/_expected/umd.js new file mode 100644 index 0000000..7f83fb9 --- /dev/null +++ b/test/form/side-effect-c/_expected/umd.js @@ -0,0 +1,11 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() : + typeof define === 'function' && define.amd ? define(factory) : + global.myBundle = factory(); +}(this, function () { 'use strict'; + + var main = 42; + + return main; + +})); \ No newline at end of file diff --git a/test/form/side-effect-c/main.js b/test/form/side-effect-c/main.js new file mode 100644 index 0000000..87ad109 --- /dev/null +++ b/test/form/side-effect-c/main.js @@ -0,0 +1,10 @@ +var factory = function () { + function Unused () {} + Unused.prototype = {}; + + return Unused; +}; + +var Unused = factory(); + +export default 42; diff --git a/test/form/side-effect-d/_config.js b/test/form/side-effect-d/_config.js new file mode 100644 index 0000000..a8d5bf9 --- /dev/null +++ b/test/form/side-effect-d/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: 'excludes functions that are known to be pure', + options: { + moduleName: 'myBundle' + } +}; diff --git a/test/form/side-effect-d/_expected/amd.js b/test/form/side-effect-d/_expected/amd.js new file mode 100644 index 0000000..37d2571 --- /dev/null +++ b/test/form/side-effect-d/_expected/amd.js @@ -0,0 +1,7 @@ +define(function () { 'use strict'; + + var main = 42; + + return main; + +}); diff --git a/test/form/side-effect-d/_expected/cjs.js b/test/form/side-effect-d/_expected/cjs.js new file mode 100644 index 0000000..5a370cd --- /dev/null +++ b/test/form/side-effect-d/_expected/cjs.js @@ -0,0 +1,5 @@ +'use strict'; + +var main = 42; + +module.exports = main; diff --git a/test/form/side-effect-d/_expected/es6.js b/test/form/side-effect-d/_expected/es6.js new file mode 100644 index 0000000..d862de8 --- /dev/null +++ b/test/form/side-effect-d/_expected/es6.js @@ -0,0 +1,3 @@ +var main = 42; + +export default main; diff --git a/test/form/side-effect-d/_expected/iife.js b/test/form/side-effect-d/_expected/iife.js new file mode 100644 index 0000000..b7b15ee --- /dev/null +++ b/test/form/side-effect-d/_expected/iife.js @@ -0,0 +1,7 @@ +var myBundle = (function () { 'use strict'; + + var main = 42; + + return main; + +})(); diff --git a/test/form/side-effect-d/_expected/umd.js b/test/form/side-effect-d/_expected/umd.js new file mode 100644 index 0000000..b5aa08a --- /dev/null +++ b/test/form/side-effect-d/_expected/umd.js @@ -0,0 +1,11 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() : + typeof define === 'function' && define.amd ? define(factory) : + global.myBundle = factory(); +}(this, function () { 'use strict'; + + var main = 42; + + return main; + +})); diff --git a/test/form/side-effect-d/main.js b/test/form/side-effect-d/main.js new file mode 100644 index 0000000..c53646d --- /dev/null +++ b/test/form/side-effect-d/main.js @@ -0,0 +1,4 @@ +var obj = { foo: 1, bar: 2 }; +var keys = Object.keys( obj ); + +export default 42; diff --git a/test/form/unused-side-effect/_config.js b/test/form/side-effect/_config.js similarity index 100% rename from test/form/unused-side-effect/_config.js rename to test/form/side-effect/_config.js diff --git a/test/form/unused-side-effect/_expected/amd.js b/test/form/side-effect/_expected/amd.js similarity index 100% rename from test/form/unused-side-effect/_expected/amd.js rename to test/form/side-effect/_expected/amd.js diff --git a/test/form/unused-side-effect/_expected/cjs.js b/test/form/side-effect/_expected/cjs.js similarity index 100% rename from test/form/unused-side-effect/_expected/cjs.js rename to test/form/side-effect/_expected/cjs.js diff --git a/test/form/unused-side-effect/_expected/es6.js b/test/form/side-effect/_expected/es6.js similarity index 100% rename from test/form/unused-side-effect/_expected/es6.js rename to test/form/side-effect/_expected/es6.js diff --git a/test/form/unused-side-effect/_expected/iife.js b/test/form/side-effect/_expected/iife.js similarity index 100% rename from test/form/unused-side-effect/_expected/iife.js rename to test/form/side-effect/_expected/iife.js diff --git a/test/form/unused-side-effect/_expected/umd.js b/test/form/side-effect/_expected/umd.js similarity index 100% rename from test/form/unused-side-effect/_expected/umd.js rename to test/form/side-effect/_expected/umd.js diff --git a/test/form/unused-side-effect/foo.js b/test/form/side-effect/foo.js similarity index 100% rename from test/form/unused-side-effect/foo.js rename to test/form/side-effect/foo.js diff --git a/test/form/unused-side-effect/main.js b/test/form/side-effect/main.js similarity index 100% rename from test/form/unused-side-effect/main.js rename to test/form/side-effect/main.js diff --git a/test/form/unmodified-default-exports-function-argument/_expected/amd.js b/test/form/unmodified-default-exports-function-argument/_expected/amd.js index c1053c2..be41871 100644 --- a/test/form/unmodified-default-exports-function-argument/_expected/amd.js +++ b/test/form/unmodified-default-exports-function-argument/_expected/amd.js @@ -11,4 +11,6 @@ define(function () { 'use strict'; var answer = foo(); var somethingElse = bar(); + console.log( answer ); + }); diff --git a/test/form/unmodified-default-exports-function-argument/_expected/cjs.js b/test/form/unmodified-default-exports-function-argument/_expected/cjs.js index 18131b8..362f677 100644 --- a/test/form/unmodified-default-exports-function-argument/_expected/cjs.js +++ b/test/form/unmodified-default-exports-function-argument/_expected/cjs.js @@ -10,3 +10,5 @@ function bar () { var answer = foo(); var somethingElse = bar(); + +console.log( answer ); diff --git a/test/form/unmodified-default-exports-function-argument/_expected/es6.js b/test/form/unmodified-default-exports-function-argument/_expected/es6.js index 6bde701..76d6f0c 100644 --- a/test/form/unmodified-default-exports-function-argument/_expected/es6.js +++ b/test/form/unmodified-default-exports-function-argument/_expected/es6.js @@ -8,3 +8,5 @@ function bar () { var answer = foo(); var somethingElse = bar(); + +console.log( answer ); diff --git a/test/form/unmodified-default-exports-function-argument/_expected/iife.js b/test/form/unmodified-default-exports-function-argument/_expected/iife.js index 8d5d37f..9fb882d 100644 --- a/test/form/unmodified-default-exports-function-argument/_expected/iife.js +++ b/test/form/unmodified-default-exports-function-argument/_expected/iife.js @@ -11,4 +11,6 @@ var answer = foo(); var somethingElse = bar(); + console.log( answer ); + })(); diff --git a/test/form/unmodified-default-exports-function-argument/_expected/umd.js b/test/form/unmodified-default-exports-function-argument/_expected/umd.js index 5ae4301..2a4c64c 100644 --- a/test/form/unmodified-default-exports-function-argument/_expected/umd.js +++ b/test/form/unmodified-default-exports-function-argument/_expected/umd.js @@ -15,4 +15,6 @@ var answer = foo(); var somethingElse = bar(); + console.log( answer ); + })); diff --git a/test/form/unmodified-default-exports-function-argument/main.js b/test/form/unmodified-default-exports-function-argument/main.js index 1cc9361..efe5d3f 100644 --- a/test/form/unmodified-default-exports-function-argument/main.js +++ b/test/form/unmodified-default-exports-function-argument/main.js @@ -2,3 +2,5 @@ import foo, { bar } from './foo'; var answer = foo(); var somethingElse = bar(); + +console.log( answer ); From e3e5846dc5a9b3419901987f1af5e3e6f4e39e45 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 17:34:43 -0400 Subject: [PATCH 03/29] tidy up --- src/Declaration.js | 6 +----- src/Statement.js | 4 +--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Declaration.js b/src/Declaration.js index 7fb7c86..47b05cb 100644 --- a/src/Declaration.js +++ b/src/Declaration.js @@ -17,7 +17,6 @@ export default class Declaration { this.name = null; this.isReassigned = false; - this.mutations = []; this.aliases = []; } @@ -30,9 +29,6 @@ export default class Declaration { this.name = reference.name; // TODO handle differences of opinion if ( reference.isReassignment ) this.isReassigned = true; - if ( reference.isMutation && !~this.mutations.indexOf( reference.statement ) ) { - this.mutations.push( reference.statement ); - } } hasSideEffect () { @@ -85,7 +81,7 @@ export class SyntheticDefaultDeclaration { if ( this.original ) { return this.original.hasSideEffect(); } - + if ( /FunctionExpression/.test( this.node.declaration.type ) ) { return testForSideEffects( this.node.declaration.body, this.statement.scope, this.statement ); } diff --git a/src/Statement.js b/src/Statement.js index 31b8bd7..1024ca6 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -112,10 +112,9 @@ export default class Statement { return this.skip(); } - const isMutation = parent && parent.type in modifierNodes; let isReassignment; - if ( isMutation ) { + if ( parent && parent.type in modifierNodes ) { let subject = parent[ modifierNodes[ parent.type ] ]; let depth = 0; @@ -154,7 +153,6 @@ export default class Statement { reference.isImmediatelyUsed = !readDepth; reference.isReassignment = isReassignment; - reference.isMutation = !readDepth && isMutation; this.skip(); // don't descend from `foo.bar.baz` into `foo.bar` } From baed9a020977dfc2d832b21d5ac4fbd2eecaada1 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 19:04:46 -0400 Subject: [PATCH 04/29] the tests all pass! but the code is a mess --- src/Declaration.js | 11 +++--- src/Module.js | 47 +++++++++++++++--------- src/Statement.js | 49 ++++++++++++------------- src/ast/isFunctionDeclaration.js | 6 ++++ src/ast/isReference.js | 21 +++++++++++ src/utils/testForSideEffects.js | 62 +++++++++++++++++--------------- 6 files changed, 120 insertions(+), 76 deletions(-) create mode 100644 src/ast/isFunctionDeclaration.js create mode 100644 src/ast/isReference.js diff --git a/src/Declaration.js b/src/Declaration.js index 47b05cb..f7195b4 100644 --- a/src/Declaration.js +++ b/src/Declaration.js @@ -31,8 +31,9 @@ export default class Declaration { if ( reference.isReassignment ) this.isReassigned = true; } - hasSideEffect () { - return testForSideEffects( this.functionBody, this.statement.scope, this.statement ); + testForSideEffects ( strongDependencies ) { + if ( !this.statement ) return; + return testForSideEffects( this.functionBody, this.statement.scope, this.statement, strongDependencies ); } render ( es6 ) { @@ -77,13 +78,13 @@ export class SyntheticDefaultDeclaration { this.original = declaration; } - hasSideEffect () { + testForSideEffects ( strongDependencies ) { if ( this.original ) { - return this.original.hasSideEffect(); + return this.original.testForSideEffects( strongDependencies ); } if ( /FunctionExpression/.test( this.node.declaration.type ) ) { - return testForSideEffects( this.node.declaration.body, this.statement.scope, this.statement ); + return testForSideEffects( this.node.declaration.body, this.statement.scope, this.statement, strongDependencies ); } } diff --git a/src/Module.js b/src/Module.js index 1e5a97b..b21b37d 100644 --- a/src/Module.js +++ b/src/Module.js @@ -49,6 +49,8 @@ export default class Module { this.declarations = blank(); this.analyse(); + + this.strongDependencies = []; } addExport ( statement ) { @@ -154,7 +156,7 @@ export default class Module { if ( statement.isImportDeclaration ) this.addImport( statement ); else if ( statement.isExportDeclaration ) this.addExport( statement ); - statement.analyse(); + statement.firstPass(); statement.scope.eachDeclaration( ( name, declaration ) => { this.declarations[ name ] = declaration; @@ -246,23 +248,29 @@ export default class Module { } }); - // identify strong dependencies to break ties in case of cycles - this.statements.forEach( statement => { - statement.references.forEach( reference => { - const declaration = reference.declaration; - - if ( declaration && declaration.statement ) { - const module = declaration.statement.module; - if ( module === this ) return; - - // TODO disregard function declarations - if ( reference.isImmediatelyUsed ) { - strongDependencies[ module.id ] = module; - } - } - }); + this.strongDependencies.forEach( module => { + if ( module !== this ) { + strongDependencies[ module.id ] = module; + } }); + // // identify strong dependencies to break ties in case of cycles + // this.statements.forEach( statement => { + // statement.references.forEach( reference => { + // const declaration = reference.declaration; + // + // if ( declaration && declaration.statement ) { + // const module = declaration.statement.module; + // if ( module === this ) return; + // + // // TODO disregard function declarations + // if ( reference.isImmediatelyUsed ) { + // strongDependencies[ module.id ] = module; + // } + // } + // }); + // }); + return { strongDependencies, weakDependencies }; } @@ -287,9 +295,14 @@ export default class Module { } markAllSideEffects () { + // console.group( this.id ) + this.statements.forEach( statement => { - statement.markSideEffect(); + statement.secondPass( this.strongDependencies ); }); + + // console.log( this.strongDependencies.map(m=>m.id) ) + // console.groupEnd() } namespace () { diff --git a/src/Statement.js b/src/Statement.js index 1024ca6..fe5c7b7 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -2,31 +2,11 @@ import { walk } from 'estree-walker'; import Scope from './ast/Scope.js'; import attachScopes from './ast/attachScopes.js'; import modifierNodes from './ast/modifierNodes.js'; +import isFunctionDeclaration from './ast/isFunctionDeclaration.js'; +import isReference from './ast/isReference.js'; import getLocation from './utils/getLocation.js'; import testForSideEffects from './utils/testForSideEffects.js'; -function isReference ( node, parent ) { - if ( node.type === 'MemberExpression' ) { - return !node.computed && isReference( node.object, node ); - } - - if ( node.type === 'Identifier' ) { - // TODO is this right? - if ( parent.type === 'MemberExpression' ) return parent.computed || node === parent.object; - - // disregard the `bar` in { bar: foo } - if ( parent.type === 'Property' && node !== parent.value ) return false; - - // disregard the `bar` in `class Foo { bar () {...} }` - if ( parent.type === 'MethodDefinition' ) return false; - - // disregard the `bar` in `export { foo as bar }` - if ( parent.type === 'ExportSpecifier' && node !== parent.local ) return; - - return true; - } -} - class Reference { constructor ( node, scope, statement ) { this.node = node; @@ -69,9 +49,12 @@ export default class Statement { this.isImportDeclaration = node.type === 'ImportDeclaration'; this.isExportDeclaration = /^Export/.test( node.type ); this.isReexportDeclaration = this.isExportDeclaration && !!node.source; + + this.isFunctionDeclaration = isFunctionDeclaration( node ) || + this.isExportDeclaration && isFunctionDeclaration( node.declaration ); } - analyse () { + firstPass () { if ( this.isImportDeclaration ) return; // nothing to analyse // attach scopes @@ -173,13 +156,27 @@ export default class Statement { }); } - markSideEffect () { - if ( this.isIncluded ) return; + secondPass ( strongDependencies ) { + // console.group( 'second pass: %s', this.toString() ) + // console.log( 'this.isIncluded', this.isIncluded ) + // console.log( 'this.isImportDeclaration', this.isImportDeclaration ) + // console.log( 'this.isFunctionDeclaration', this.isFunctionDeclaration ) - if ( testForSideEffects( this.node, this.scope, this ) ) { + if ( this.didSecondPassAlready || this.isImportDeclaration || this.isFunctionDeclaration ) { + // console.log( '>>> skipping' ) + // console.groupEnd() + return; + } + + this.didSecondPassAlready = true; + + if ( testForSideEffects( this.node, this.scope, this, strongDependencies ) ) { this.mark(); + // console.groupEnd() return true; } + + // console.groupEnd() } source () { diff --git a/src/ast/isFunctionDeclaration.js b/src/ast/isFunctionDeclaration.js new file mode 100644 index 0000000..02af498 --- /dev/null +++ b/src/ast/isFunctionDeclaration.js @@ -0,0 +1,6 @@ +export default function isFunctionDeclaration ( node ) { + if ( !node ) return false; + + return node.type === 'FunctionDeclaration' || + ( node.type === 'VariableDeclaration' && node.init && /FunctionExpression/.test( node.init.type ) ); +} diff --git a/src/ast/isReference.js b/src/ast/isReference.js new file mode 100644 index 0000000..7e0c8db --- /dev/null +++ b/src/ast/isReference.js @@ -0,0 +1,21 @@ +export default function isReference ( node, parent ) { + if ( node.type === 'MemberExpression' ) { + return !node.computed && isReference( node.object, node ); + } + + if ( node.type === 'Identifier' ) { + // TODO is this right? + if ( parent.type === 'MemberExpression' ) return parent.computed || node === parent.object; + + // disregard the `bar` in { bar: foo } + if ( parent.type === 'Property' && node !== parent.value ) return false; + + // disregard the `bar` in `class Foo { bar () {...} }` + if ( parent.type === 'MethodDefinition' ) return false; + + // disregard the `bar` in `export { foo as bar }` + if ( parent.type === 'ExportSpecifier' && node !== parent.local ) return; + + return true; + } +} diff --git a/src/utils/testForSideEffects.js b/src/utils/testForSideEffects.js index 1aa4f96..9df2ef4 100644 --- a/src/utils/testForSideEffects.js +++ b/src/utils/testForSideEffects.js @@ -1,5 +1,7 @@ import { walk } from 'estree-walker'; import modifierNodes from '../ast/modifierNodes.js'; +import isFunctionDeclaration from '../ast/isFunctionDeclaration.js'; +import isReference from '../ast/isReference.js'; import flatten from '../ast/flatten'; let pureFunctions = {}; @@ -8,70 +10,75 @@ let pureFunctions = {}; 'Object.keys' ].forEach( name => pureFunctions[ name ] = true ); -export default function testForSideEffects ( node, scope, statement ) { +export default function testForSideEffects ( node, scope, statement, strongDependencies, force ) { let hasSideEffect = false; walk( node, { - enter ( node ) { - if ( hasSideEffect ) return this.skip(); - if ( /Function/.test( node.type ) ) return this.skip(); + enter ( node, parent ) { + if ( !force && /Function/.test( node.type ) ) return this.skip(); if ( node._scope ) scope = node._scope; + if ( isReference( node, parent ) ) { + const flattened = flatten( node ); + + if ( !scope.contains( flattened.name ) ) { + const declaration = statement.module.trace( flattened.name ); + if ( declaration && !declaration.isExternal ) { + const module = declaration.module || declaration.statement.module; // TODO is this right? + if ( !~strongDependencies.indexOf( module ) ) strongDependencies.push( module ); + } + } + } + // If this is a top-level call expression, or an assignment to a global, // this statement will need to be marked - if ( node.type === 'NewExpression' ) { + else if ( node.type === 'NewExpression' ) { hasSideEffect = true; - return this.skip(); } - if ( node.type === 'CallExpression' ) { - if ( node.callee.type === 'Identifier' && !scope.contains( node.callee.name ) ) { - const declaration = statement.module.trace( node.callee.name ); + else if ( node.type === 'CallExpression' ) { + if ( node.callee.type === 'Identifier' ) { + const declaration = scope.findDeclaration( node.callee.name ) || + statement.module.trace( node.callee.name ); if ( !declaration || declaration.isExternal ) { // we're calling a global or an external function. Assume side-effects hasSideEffect = true; - return this.skip(); } // we're calling a function defined in this bundle - if ( declaration.hasSideEffect() ) { + else if ( declaration.testForSideEffects( strongDependencies ) ) { hasSideEffect = true; - return this.skip(); } // TODO does function mutate inputs that are needed? - return; } - if ( node.callee.type === 'MemberExpression' ) { + else if ( node.callee.type === 'MemberExpression' ) { const flattened = flatten( node.callee ); - if ( !flattened ) { + if ( flattened ) { + // if we're calling e.g. Object.keys(thing), there are no side-effects + // TODO make pureFunctions configurable + const declaration = statement.module.trace( flattened.name ); + if ( !!declaration || !pureFunctions[ flattened.keypath ] ) { + hasSideEffect = true; + } + } else { // is not a keypath like `foo.bar.baz` – could be e.g. // `(a || b).foo()`. Err on the side of caution hasSideEffect = true; - return; } - - // if we're calling e.g. Object.keys(thing), there are no side-effects - // TODO make pureFunctions configurable - const declaration = statement.module.trace( flattened.name ); - if ( !declaration && pureFunctions[ flattened.keypath ] ) return; - - hasSideEffect = true; - return this.skip(); } // otherwise we're probably dealing with a function expression - if ( testForSideEffects( node.callee, scope, statement ) ) { + else if ( testForSideEffects( node.callee, scope, statement, strongDependencies, true ) ) { hasSideEffect = true; - return this.skip(); } } - if ( node.type in modifierNodes ) { + else if ( node.type in modifierNodes ) { let subject = node[ modifierNodes[ node.type ] ]; while ( subject.type === 'MemberExpression' ) subject = subject.object; @@ -79,7 +86,6 @@ export default function testForSideEffects ( node, scope, statement ) { if ( !declaration || declaration.isExternal || declaration.statement.isIncluded ) { hasSideEffect = true; - return this.skip(); } } }, From 38374f1f93e28267fbd5166814f46b1f946dab60 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 19:10:10 -0400 Subject: [PATCH 05/29] tidy up --- src/Bundle.js | 5 ++--- src/Module.js | 25 ++----------------------- src/Statement.js | 16 +--------------- src/ast/isFunctionDeclaration.js | 2 +- 4 files changed, 6 insertions(+), 42 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index 8bc5826..4130909 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -73,9 +73,8 @@ export default class Bundle { declaration.use(); }); - this.modules.forEach( module => { - module.markAllSideEffects(); - }); + // mark statements that should appear in the bundle + this.modules.forEach( module => module.markStatements() ); this.orderedModules = this.sort(); this.deconflict(); diff --git a/src/Module.js b/src/Module.js index b21b37d..b777ea6 100644 --- a/src/Module.js +++ b/src/Module.js @@ -244,6 +244,7 @@ export default class Module { const dependency = this.bundle.moduleById[ id ]; if ( !dependency.isExternal ) { + // TODO this is a weird data structure weakDependencies[ dependency.id ] = dependency; } }); @@ -254,23 +255,6 @@ export default class Module { } }); - // // identify strong dependencies to break ties in case of cycles - // this.statements.forEach( statement => { - // statement.references.forEach( reference => { - // const declaration = reference.declaration; - // - // if ( declaration && declaration.statement ) { - // const module = declaration.statement.module; - // if ( module === this ) return; - // - // // TODO disregard function declarations - // if ( reference.isImmediatelyUsed ) { - // strongDependencies[ module.id ] = module; - // } - // } - // }); - // }); - return { strongDependencies, weakDependencies }; } @@ -294,15 +278,10 @@ export default class Module { return keys( exports ); } - markAllSideEffects () { - // console.group( this.id ) - + markStatements () { this.statements.forEach( statement => { statement.secondPass( this.strongDependencies ); }); - - // console.log( this.strongDependencies.map(m=>m.id) ) - // console.groupEnd() } namespace () { diff --git a/src/Statement.js b/src/Statement.js index fe5c7b7..00e0fcc 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -157,26 +157,12 @@ export default class Statement { } secondPass ( strongDependencies ) { - // console.group( 'second pass: %s', this.toString() ) - // console.log( 'this.isIncluded', this.isIncluded ) - // console.log( 'this.isImportDeclaration', this.isImportDeclaration ) - // console.log( 'this.isFunctionDeclaration', this.isFunctionDeclaration ) - - if ( this.didSecondPassAlready || this.isImportDeclaration || this.isFunctionDeclaration ) { - // console.log( '>>> skipping' ) - // console.groupEnd() - return; - } - - this.didSecondPassAlready = true; + if ( this.isImportDeclaration || this.isFunctionDeclaration ) return; if ( testForSideEffects( this.node, this.scope, this, strongDependencies ) ) { this.mark(); - // console.groupEnd() return true; } - - // console.groupEnd() } source () { diff --git a/src/ast/isFunctionDeclaration.js b/src/ast/isFunctionDeclaration.js index 02af498..a1573e7 100644 --- a/src/ast/isFunctionDeclaration.js +++ b/src/ast/isFunctionDeclaration.js @@ -1,6 +1,6 @@ export default function isFunctionDeclaration ( node ) { if ( !node ) return false; - + return node.type === 'FunctionDeclaration' || ( node.type === 'VariableDeclaration' && node.init && /FunctionExpression/.test( node.init.type ) ); } From 5024584f2dcfe309c0e946c2d1e600bfe8b79b5d Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 21:09:13 -0400 Subject: [PATCH 06/29] add all variable declarators (not split below top level) --- src/ast/attachScopes.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ast/attachScopes.js b/src/ast/attachScopes.js index 77b8af4..ab166ad 100644 --- a/src/ast/attachScopes.js +++ b/src/ast/attachScopes.js @@ -20,8 +20,9 @@ export default function attachScopes ( statement ) { // var foo = 1 if ( node.type === 'VariableDeclaration' ) { const isBlockDeclaration = blockDeclarations[ node.kind ]; - // only one declarator per block, because we split them up already - scope.addDeclaration( node.declarations[0], isBlockDeclaration, true ); + node.declarations.forEach( declarator => { + scope.addDeclaration( declarator, isBlockDeclaration, true ); + }); } let newScope; From 1ffd5bfae750d96880311ab19cf6440d39b3814e Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 21:11:23 -0400 Subject: [PATCH 07/29] use correct scope for testing declarations --- src/Declaration.js | 8 ++++---- src/ast/attachScopes.js | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Declaration.js b/src/Declaration.js index f7195b4..0abc9af 100644 --- a/src/Declaration.js +++ b/src/Declaration.js @@ -6,10 +6,10 @@ export default class Declaration { if ( node ) { if ( node.type === 'FunctionDeclaration' ) { this.isFunctionDeclaration = true; - this.functionBody = node.body; + this.functionNode = node; } else if ( node.type === 'VariableDeclarator' && node.init && /FunctionExpression/.test( node.init.type ) ) { this.isFunctionDeclaration = true; - this.functionBody = node.init.body; + this.functionNode = node.init; } } @@ -32,8 +32,8 @@ export default class Declaration { } testForSideEffects ( strongDependencies ) { - if ( !this.statement ) return; - return testForSideEffects( this.functionBody, this.statement.scope, this.statement, strongDependencies ); + if ( !this.statement || !this.functionNode ) return; + return testForSideEffects( this.functionNode.body, this.functionNode._scope, this.statement, strongDependencies ); } render ( es6 ) { diff --git a/src/ast/attachScopes.js b/src/ast/attachScopes.js index ab166ad..9c12df0 100644 --- a/src/ast/attachScopes.js +++ b/src/ast/attachScopes.js @@ -20,6 +20,7 @@ export default function attachScopes ( statement ) { // var foo = 1 if ( node.type === 'VariableDeclaration' ) { const isBlockDeclaration = blockDeclarations[ node.kind ]; + node.declarations.forEach( declarator => { scope.addDeclaration( declarator, isBlockDeclaration, true ); }); From 1dc3669b52a6493d723a04fdb1521e0e79e51439 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 21:13:17 -0400 Subject: [PATCH 08/29] ignore changes local to a function when determining global side-effects --- src/utils/testForSideEffects.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/utils/testForSideEffects.js b/src/utils/testForSideEffects.js index 9df2ef4..93d8cb8 100644 --- a/src/utils/testForSideEffects.js +++ b/src/utils/testForSideEffects.js @@ -82,10 +82,12 @@ export default function testForSideEffects ( node, scope, statement, strongDepen let subject = node[ modifierNodes[ node.type ] ]; while ( subject.type === 'MemberExpression' ) subject = subject.object; - const declaration = statement.module.trace( subject.name ); + if ( !scope.findDeclaration( subject.name ) ) { + const declaration = statement.module.trace( subject.name ); - if ( !declaration || declaration.isExternal || declaration.statement.isIncluded ) { - hasSideEffect = true; + if ( !declaration || declaration.isExternal || declaration.isUsed ) { + hasSideEffect = true; + } } } }, From 37f79e2836455fd986fa36f6b6a6c1e9f821ea63 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 21:35:12 -0400 Subject: [PATCH 09/29] add test --- test/form/side-effect-e/_config.js | 7 +++++++ test/form/side-effect-e/_expected/amd.js | 7 +++++++ test/form/side-effect-e/_expected/cjs.js | 5 +++++ test/form/side-effect-e/_expected/es6.js | 3 +++ test/form/side-effect-e/_expected/iife.js | 7 +++++++ test/form/side-effect-e/_expected/umd.js | 11 +++++++++++ test/form/side-effect-e/main.js | 8 ++++++++ 7 files changed, 48 insertions(+) create mode 100644 test/form/side-effect-e/_config.js create mode 100644 test/form/side-effect-e/_expected/amd.js create mode 100644 test/form/side-effect-e/_expected/cjs.js create mode 100644 test/form/side-effect-e/_expected/es6.js create mode 100644 test/form/side-effect-e/_expected/iife.js create mode 100644 test/form/side-effect-e/_expected/umd.js create mode 100644 test/form/side-effect-e/main.js diff --git a/test/form/side-effect-e/_config.js b/test/form/side-effect-e/_config.js new file mode 100644 index 0000000..456d304 --- /dev/null +++ b/test/form/side-effect-e/_config.js @@ -0,0 +1,7 @@ +module.exports = { + solo: true, + description: 'disregards side-effects that are contained within a function', + options: { + moduleName: 'myBundle' + } +}; diff --git a/test/form/side-effect-e/_expected/amd.js b/test/form/side-effect-e/_expected/amd.js new file mode 100644 index 0000000..37d2571 --- /dev/null +++ b/test/form/side-effect-e/_expected/amd.js @@ -0,0 +1,7 @@ +define(function () { 'use strict'; + + var main = 42; + + return main; + +}); diff --git a/test/form/side-effect-e/_expected/cjs.js b/test/form/side-effect-e/_expected/cjs.js new file mode 100644 index 0000000..5a370cd --- /dev/null +++ b/test/form/side-effect-e/_expected/cjs.js @@ -0,0 +1,5 @@ +'use strict'; + +var main = 42; + +module.exports = main; diff --git a/test/form/side-effect-e/_expected/es6.js b/test/form/side-effect-e/_expected/es6.js new file mode 100644 index 0000000..d862de8 --- /dev/null +++ b/test/form/side-effect-e/_expected/es6.js @@ -0,0 +1,3 @@ +var main = 42; + +export default main; diff --git a/test/form/side-effect-e/_expected/iife.js b/test/form/side-effect-e/_expected/iife.js new file mode 100644 index 0000000..b7b15ee --- /dev/null +++ b/test/form/side-effect-e/_expected/iife.js @@ -0,0 +1,7 @@ +var myBundle = (function () { 'use strict'; + + var main = 42; + + return main; + +})(); diff --git a/test/form/side-effect-e/_expected/umd.js b/test/form/side-effect-e/_expected/umd.js new file mode 100644 index 0000000..b5aa08a --- /dev/null +++ b/test/form/side-effect-e/_expected/umd.js @@ -0,0 +1,11 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() : + typeof define === 'function' && define.amd ? define(factory) : + global.myBundle = factory(); +}(this, function () { 'use strict'; + + var main = 42; + + return main; + +})); diff --git a/test/form/side-effect-e/main.js b/test/form/side-effect-e/main.js new file mode 100644 index 0000000..e75b2f3 --- /dev/null +++ b/test/form/side-effect-e/main.js @@ -0,0 +1,8 @@ +function foo () { + var a, b, c; + b = 1; +} + +foo(); + +export default 42; From d997bf16a77d0489b14f4f09a7660b6f305423df Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 21:48:57 -0400 Subject: [PATCH 10/29] tidy up --- src/utils/testForSideEffects.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/utils/testForSideEffects.js b/src/utils/testForSideEffects.js index 93d8cb8..12beb3f 100644 --- a/src/utils/testForSideEffects.js +++ b/src/utils/testForSideEffects.js @@ -42,6 +42,14 @@ export default function testForSideEffects ( node, scope, statement, strongDepen const declaration = scope.findDeclaration( node.callee.name ) || statement.module.trace( node.callee.name ); + if ( declaration ) { + if ( declaration.isExternal || declaration.testForSideEffects( strongDependencies ) ) { + hasSideEffect = true; + } + } else if ( !pureFunctions[ node.callee.name ] ) { + hasSideEffect = true; + } + if ( !declaration || declaration.isExternal ) { // we're calling a global or an external function. Assume side-effects hasSideEffect = true; From fd0cbb7e76d372dceb21300d0077efb4b44ad13c Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 22:03:23 -0400 Subject: [PATCH 11/29] account for local scopes when checking for pure global functions --- src/utils/testForSideEffects.js | 3 ++- test/form/side-effect-e/_config.js | 3 +-- test/form/side-effect-e/_expected/amd.js | 13 +++++++++++++ test/form/side-effect-e/_expected/cjs.js | 13 +++++++++++++ test/form/side-effect-e/_expected/es6.js | 13 +++++++++++++ test/form/side-effect-e/_expected/iife.js | 13 +++++++++++++ test/form/side-effect-e/_expected/umd.js | 13 +++++++++++++ test/form/side-effect-e/main.js | 10 ++++++++-- 8 files changed, 76 insertions(+), 5 deletions(-) diff --git a/src/utils/testForSideEffects.js b/src/utils/testForSideEffects.js index 12beb3f..6251c83 100644 --- a/src/utils/testForSideEffects.js +++ b/src/utils/testForSideEffects.js @@ -69,7 +69,8 @@ export default function testForSideEffects ( node, scope, statement, strongDepen if ( flattened ) { // if we're calling e.g. Object.keys(thing), there are no side-effects // TODO make pureFunctions configurable - const declaration = statement.module.trace( flattened.name ); + const declaration = scope.findDeclaration( flattened.name ) || statement.module.trace( flattened.name ); + if ( !!declaration || !pureFunctions[ flattened.keypath ] ) { hasSideEffect = true; } diff --git a/test/form/side-effect-e/_config.js b/test/form/side-effect-e/_config.js index 456d304..a8d5bf9 100644 --- a/test/form/side-effect-e/_config.js +++ b/test/form/side-effect-e/_config.js @@ -1,6 +1,5 @@ module.exports = { - solo: true, - description: 'disregards side-effects that are contained within a function', + description: 'excludes functions that are known to be pure', options: { moduleName: 'myBundle' } diff --git a/test/form/side-effect-e/_expected/amd.js b/test/form/side-effect-e/_expected/amd.js index 37d2571..049c368 100644 --- a/test/form/side-effect-e/_expected/amd.js +++ b/test/form/side-effect-e/_expected/amd.js @@ -1,5 +1,18 @@ define(function () { 'use strict'; + function foo () { + var Object = { + keys: function () { + console.log( 'side-effect' ); + } + }; + + var obj = { foo: 1, bar: 2 }; + var keys = Object.keys( obj ); + } + + foo(); + var main = 42; return main; diff --git a/test/form/side-effect-e/_expected/cjs.js b/test/form/side-effect-e/_expected/cjs.js index 5a370cd..9f85382 100644 --- a/test/form/side-effect-e/_expected/cjs.js +++ b/test/form/side-effect-e/_expected/cjs.js @@ -1,5 +1,18 @@ 'use strict'; +function foo () { + var Object = { + keys: function () { + console.log( 'side-effect' ); + } + }; + + var obj = { foo: 1, bar: 2 }; + var keys = Object.keys( obj ); +} + +foo(); + var main = 42; module.exports = main; diff --git a/test/form/side-effect-e/_expected/es6.js b/test/form/side-effect-e/_expected/es6.js index d862de8..eaaa99a 100644 --- a/test/form/side-effect-e/_expected/es6.js +++ b/test/form/side-effect-e/_expected/es6.js @@ -1,3 +1,16 @@ +function foo () { + var Object = { + keys: function () { + console.log( 'side-effect' ); + } + }; + + var obj = { foo: 1, bar: 2 }; + var keys = Object.keys( obj ); +} + +foo(); + var main = 42; export default main; diff --git a/test/form/side-effect-e/_expected/iife.js b/test/form/side-effect-e/_expected/iife.js index b7b15ee..5757003 100644 --- a/test/form/side-effect-e/_expected/iife.js +++ b/test/form/side-effect-e/_expected/iife.js @@ -1,5 +1,18 @@ var myBundle = (function () { 'use strict'; + function foo () { + var Object = { + keys: function () { + console.log( 'side-effect' ); + } + }; + + var obj = { foo: 1, bar: 2 }; + var keys = Object.keys( obj ); + } + + foo(); + var main = 42; return main; diff --git a/test/form/side-effect-e/_expected/umd.js b/test/form/side-effect-e/_expected/umd.js index b5aa08a..12803ec 100644 --- a/test/form/side-effect-e/_expected/umd.js +++ b/test/form/side-effect-e/_expected/umd.js @@ -4,6 +4,19 @@ global.myBundle = factory(); }(this, function () { 'use strict'; + function foo () { + var Object = { + keys: function () { + console.log( 'side-effect' ); + } + }; + + var obj = { foo: 1, bar: 2 }; + var keys = Object.keys( obj ); + } + + foo(); + var main = 42; return main; diff --git a/test/form/side-effect-e/main.js b/test/form/side-effect-e/main.js index e75b2f3..b96dfdf 100644 --- a/test/form/side-effect-e/main.js +++ b/test/form/side-effect-e/main.js @@ -1,6 +1,12 @@ function foo () { - var a, b, c; - b = 1; + var Object = { + keys: function () { + console.log( 'side-effect' ); + } + }; + + var obj = { foo: 1, bar: 2 }; + var keys = Object.keys( obj ); } foo(); From dec7c8482afedfb82c19bd48c5799483cc6c9b4b Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 22:05:01 -0400 Subject: [PATCH 12/29] remove unused code --- src/utils/testForSideEffects.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/utils/testForSideEffects.js b/src/utils/testForSideEffects.js index 6251c83..aeb2413 100644 --- a/src/utils/testForSideEffects.js +++ b/src/utils/testForSideEffects.js @@ -50,16 +50,6 @@ export default function testForSideEffects ( node, scope, statement, strongDepen hasSideEffect = true; } - if ( !declaration || declaration.isExternal ) { - // we're calling a global or an external function. Assume side-effects - hasSideEffect = true; - } - - // we're calling a function defined in this bundle - else if ( declaration.testForSideEffects( strongDependencies ) ) { - hasSideEffect = true; - } - // TODO does function mutate inputs that are needed? } From c44cf0d0e62e24cadc08f50ae3d0039b30ca9982 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 22:36:40 -0400 Subject: [PATCH 13/29] exclude constructors that are known to be pure --- src/utils/testForSideEffects.js | 14 ++++++++------ test/form/side-effect-f/_config.js | 6 ++++++ test/form/side-effect-f/_expected/amd.js | 7 +++++++ test/form/side-effect-f/_expected/cjs.js | 5 +++++ test/form/side-effect-f/_expected/es6.js | 3 +++ test/form/side-effect-f/_expected/iife.js | 7 +++++++ test/form/side-effect-f/_expected/umd.js | 11 +++++++++++ test/form/side-effect-f/main.js | 8 ++++++++ test/form/side-effect-g/_config.js | 6 ++++++ test/form/side-effect-g/_expected/amd.js | 7 +++++++ test/form/side-effect-g/_expected/cjs.js | 5 +++++ test/form/side-effect-g/_expected/es6.js | 3 +++ test/form/side-effect-g/_expected/iife.js | 7 +++++++ test/form/side-effect-g/_expected/umd.js | 11 +++++++++++ test/form/side-effect-g/main.js | 3 +++ 15 files changed, 97 insertions(+), 6 deletions(-) create mode 100644 test/form/side-effect-f/_config.js create mode 100644 test/form/side-effect-f/_expected/amd.js create mode 100644 test/form/side-effect-f/_expected/cjs.js create mode 100644 test/form/side-effect-f/_expected/es6.js create mode 100644 test/form/side-effect-f/_expected/iife.js create mode 100644 test/form/side-effect-f/_expected/umd.js create mode 100644 test/form/side-effect-f/main.js create mode 100644 test/form/side-effect-g/_config.js create mode 100644 test/form/side-effect-g/_expected/amd.js create mode 100644 test/form/side-effect-g/_expected/cjs.js create mode 100644 test/form/side-effect-g/_expected/es6.js create mode 100644 test/form/side-effect-g/_expected/iife.js create mode 100644 test/form/side-effect-g/_expected/umd.js create mode 100644 test/form/side-effect-g/main.js diff --git a/src/utils/testForSideEffects.js b/src/utils/testForSideEffects.js index aeb2413..1a42eba 100644 --- a/src/utils/testForSideEffects.js +++ b/src/utils/testForSideEffects.js @@ -6,8 +6,12 @@ import flatten from '../ast/flatten'; let pureFunctions = {}; [ - // TODO add others to this list - 'Object.keys' + // TODO add others to this list from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects + 'Array', 'Array.isArray', + + 'Error', 'EvalError', 'InternalError', 'RangeError', 'ReferenceError', 'SyntaxError', 'TypeError', 'URIError', + + 'Object', 'Object.keys' ].forEach( name => pureFunctions[ name ] = true ); export default function testForSideEffects ( node, scope, statement, strongDependencies, force ) { @@ -31,13 +35,11 @@ export default function testForSideEffects ( node, scope, statement, strongDepen } } - // If this is a top-level call expression, or an assignment to a global, - // this statement will need to be marked - else if ( node.type === 'NewExpression' ) { + else if ( node.type === 'ThrowStatement' ) { hasSideEffect = true; } - else if ( node.type === 'CallExpression' ) { + else if ( node.type === 'CallExpression' || node.type === 'NewExpression' ) { if ( node.callee.type === 'Identifier' ) { const declaration = scope.findDeclaration( node.callee.name ) || statement.module.trace( node.callee.name ); diff --git a/test/form/side-effect-f/_config.js b/test/form/side-effect-f/_config.js new file mode 100644 index 0000000..67f9fa4 --- /dev/null +++ b/test/form/side-effect-f/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: 'disregards side-effects that are contained within a function', + options: { + moduleName: 'myBundle' + } +}; diff --git a/test/form/side-effect-f/_expected/amd.js b/test/form/side-effect-f/_expected/amd.js new file mode 100644 index 0000000..37d2571 --- /dev/null +++ b/test/form/side-effect-f/_expected/amd.js @@ -0,0 +1,7 @@ +define(function () { 'use strict'; + + var main = 42; + + return main; + +}); diff --git a/test/form/side-effect-f/_expected/cjs.js b/test/form/side-effect-f/_expected/cjs.js new file mode 100644 index 0000000..5a370cd --- /dev/null +++ b/test/form/side-effect-f/_expected/cjs.js @@ -0,0 +1,5 @@ +'use strict'; + +var main = 42; + +module.exports = main; diff --git a/test/form/side-effect-f/_expected/es6.js b/test/form/side-effect-f/_expected/es6.js new file mode 100644 index 0000000..d862de8 --- /dev/null +++ b/test/form/side-effect-f/_expected/es6.js @@ -0,0 +1,3 @@ +var main = 42; + +export default main; diff --git a/test/form/side-effect-f/_expected/iife.js b/test/form/side-effect-f/_expected/iife.js new file mode 100644 index 0000000..b7b15ee --- /dev/null +++ b/test/form/side-effect-f/_expected/iife.js @@ -0,0 +1,7 @@ +var myBundle = (function () { 'use strict'; + + var main = 42; + + return main; + +})(); diff --git a/test/form/side-effect-f/_expected/umd.js b/test/form/side-effect-f/_expected/umd.js new file mode 100644 index 0000000..b5aa08a --- /dev/null +++ b/test/form/side-effect-f/_expected/umd.js @@ -0,0 +1,11 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() : + typeof define === 'function' && define.amd ? define(factory) : + global.myBundle = factory(); +}(this, function () { 'use strict'; + + var main = 42; + + return main; + +})); diff --git a/test/form/side-effect-f/main.js b/test/form/side-effect-f/main.js new file mode 100644 index 0000000..e75b2f3 --- /dev/null +++ b/test/form/side-effect-f/main.js @@ -0,0 +1,8 @@ +function foo () { + var a, b, c; + b = 1; +} + +foo(); + +export default 42; diff --git a/test/form/side-effect-g/_config.js b/test/form/side-effect-g/_config.js new file mode 100644 index 0000000..c821995 --- /dev/null +++ b/test/form/side-effect-g/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: 'excludes constructors that are known to be pure', + options: { + moduleName: 'myBundle' + } +}; diff --git a/test/form/side-effect-g/_expected/amd.js b/test/form/side-effect-g/_expected/amd.js new file mode 100644 index 0000000..37d2571 --- /dev/null +++ b/test/form/side-effect-g/_expected/amd.js @@ -0,0 +1,7 @@ +define(function () { 'use strict'; + + var main = 42; + + return main; + +}); diff --git a/test/form/side-effect-g/_expected/cjs.js b/test/form/side-effect-g/_expected/cjs.js new file mode 100644 index 0000000..5a370cd --- /dev/null +++ b/test/form/side-effect-g/_expected/cjs.js @@ -0,0 +1,5 @@ +'use strict'; + +var main = 42; + +module.exports = main; diff --git a/test/form/side-effect-g/_expected/es6.js b/test/form/side-effect-g/_expected/es6.js new file mode 100644 index 0000000..d862de8 --- /dev/null +++ b/test/form/side-effect-g/_expected/es6.js @@ -0,0 +1,3 @@ +var main = 42; + +export default main; diff --git a/test/form/side-effect-g/_expected/iife.js b/test/form/side-effect-g/_expected/iife.js new file mode 100644 index 0000000..b7b15ee --- /dev/null +++ b/test/form/side-effect-g/_expected/iife.js @@ -0,0 +1,7 @@ +var myBundle = (function () { 'use strict'; + + var main = 42; + + return main; + +})(); diff --git a/test/form/side-effect-g/_expected/umd.js b/test/form/side-effect-g/_expected/umd.js new file mode 100644 index 0000000..b5aa08a --- /dev/null +++ b/test/form/side-effect-g/_expected/umd.js @@ -0,0 +1,11 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() : + typeof define === 'function' && define.amd ? define(factory) : + global.myBundle = factory(); +}(this, function () { 'use strict'; + + var main = 42; + + return main; + +})); diff --git a/test/form/side-effect-g/main.js b/test/form/side-effect-g/main.js new file mode 100644 index 0000000..75d2cf5 --- /dev/null +++ b/test/form/side-effect-g/main.js @@ -0,0 +1,3 @@ +var err = new Error( 'this will be ignored' ); + +export default 42; From d17dbbdd0912ce9f5188f0dac232c4d3e5480dac Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 23:20:16 -0400 Subject: [PATCH 14/29] update test --- test/form/dedupes-external-imports/_config.js | 3 ++- test/form/dedupes-external-imports/_expected/amd.js | 6 +++++- test/form/dedupes-external-imports/_expected/cjs.js | 4 ++++ test/form/dedupes-external-imports/_expected/es6.js | 2 ++ test/form/dedupes-external-imports/_expected/iife.js | 8 ++++++-- test/form/dedupes-external-imports/_expected/umd.js | 12 ++++++++---- test/form/dedupes-external-imports/main.js | 6 +++--- 7 files changed, 30 insertions(+), 11 deletions(-) diff --git a/test/form/dedupes-external-imports/_config.js b/test/form/dedupes-external-imports/_config.js index ed2bff2..90dbdec 100644 --- a/test/form/dedupes-external-imports/_config.js +++ b/test/form/dedupes-external-imports/_config.js @@ -1,6 +1,7 @@ module.exports = { description: 'dedupes external imports', options: { - external: [ 'external' ] + external: [ 'external' ], + moduleName: 'myBundle' } }; diff --git a/test/form/dedupes-external-imports/_expected/amd.js b/test/form/dedupes-external-imports/_expected/amd.js index be13b6a..7f52eda 100644 --- a/test/form/dedupes-external-imports/_expected/amd.js +++ b/test/form/dedupes-external-imports/_expected/amd.js @@ -1,4 +1,4 @@ -define(['external'], function (external) { 'use strict'; +define(['exports', 'external'], function (exports, external) { 'use strict'; class Foo extends external.Component { constructor () { @@ -25,4 +25,8 @@ define(['external'], function (external) { 'use strict'; const bar = new Bar(); const baz = new Baz(); + exports.foo = foo; + exports.bar = bar; + exports.baz = baz; + }); diff --git a/test/form/dedupes-external-imports/_expected/cjs.js b/test/form/dedupes-external-imports/_expected/cjs.js index 01dda8c..fd4138b 100644 --- a/test/form/dedupes-external-imports/_expected/cjs.js +++ b/test/form/dedupes-external-imports/_expected/cjs.js @@ -26,3 +26,7 @@ class Baz extends external.Component { const foo = new Foo(); const bar = new Bar(); const baz = new Baz(); + +exports.foo = foo; +exports.bar = bar; +exports.baz = baz; diff --git a/test/form/dedupes-external-imports/_expected/es6.js b/test/form/dedupes-external-imports/_expected/es6.js index 884df6d..c405e7c 100644 --- a/test/form/dedupes-external-imports/_expected/es6.js +++ b/test/form/dedupes-external-imports/_expected/es6.js @@ -24,3 +24,5 @@ class Baz extends Component { const foo = new Foo(); const bar = new Bar(); const baz = new Baz(); + +export { foo, bar, baz }; diff --git a/test/form/dedupes-external-imports/_expected/iife.js b/test/form/dedupes-external-imports/_expected/iife.js index 2656d84..6e465c7 100644 --- a/test/form/dedupes-external-imports/_expected/iife.js +++ b/test/form/dedupes-external-imports/_expected/iife.js @@ -1,4 +1,4 @@ -(function (external) { 'use strict'; +(function (exports,external) { 'use strict'; class Foo extends external.Component { constructor () { @@ -25,4 +25,8 @@ const bar = new Bar(); const baz = new Baz(); -})(external); + exports.foo = foo; + exports.bar = bar; + exports.baz = baz; + +})((this.myBundle = {}),external); diff --git a/test/form/dedupes-external-imports/_expected/umd.js b/test/form/dedupes-external-imports/_expected/umd.js index 49ca55a..5be874c 100644 --- a/test/form/dedupes-external-imports/_expected/umd.js +++ b/test/form/dedupes-external-imports/_expected/umd.js @@ -1,8 +1,8 @@ (function (global, factory) { - typeof exports === 'object' && typeof module !== 'undefined' ? factory(require('external')) : - typeof define === 'function' && define.amd ? define(['external'], factory) : - factory(global.external); -}(this, function (external) { 'use strict'; + typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('external')) : + typeof define === 'function' && define.amd ? define(['exports', 'external'], factory) : + factory((global.myBundle = {}),global.external); +}(this, function (exports,external) { 'use strict'; class Foo extends external.Component { constructor () { @@ -29,4 +29,8 @@ const bar = new Bar(); const baz = new Baz(); + exports.foo = foo; + exports.bar = bar; + exports.baz = baz; + })); diff --git a/test/form/dedupes-external-imports/main.js b/test/form/dedupes-external-imports/main.js index 2fe3164..d029b72 100644 --- a/test/form/dedupes-external-imports/main.js +++ b/test/form/dedupes-external-imports/main.js @@ -2,6 +2,6 @@ import Foo from './foo'; import Bar from './bar'; import Baz from './baz'; -const foo = new Foo(); -const bar = new Bar(); -const baz = new Baz(); +export const foo = new Foo(); +export const bar = new Bar(); +export const baz = new Baz(); From 257403072a1a033e62c7b29c4b9671315634b657 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 23:25:51 -0400 Subject: [PATCH 15/29] include called functions that mutate their inputs --- src/Declaration.js | 4 +++- src/ast/Scope.js | 2 +- src/utils/testForSideEffects.js | 12 ++++++------ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/Declaration.js b/src/Declaration.js index 0abc9af..70f94a8 100644 --- a/src/Declaration.js +++ b/src/Declaration.js @@ -2,7 +2,7 @@ import { blank, keys } from './utils/object.js'; import testForSideEffects from './utils/testForSideEffects.js'; export default class Declaration { - constructor ( node ) { + constructor ( node, isParam ) { if ( node ) { if ( node.type === 'FunctionDeclaration' ) { this.isFunctionDeclaration = true; @@ -15,6 +15,7 @@ export default class Declaration { this.statement = null; this.name = null; + this.isParam = isParam; this.isReassigned = false; this.aliases = []; @@ -32,6 +33,7 @@ export default class Declaration { } testForSideEffects ( strongDependencies ) { + // TODO memoize this if ( !this.statement || !this.functionNode ) return; return testForSideEffects( this.functionNode.body, this.functionNode._scope, this.statement, strongDependencies ); } diff --git a/src/ast/Scope.js b/src/ast/Scope.js index a0d1350..3878c8b 100644 --- a/src/ast/Scope.js +++ b/src/ast/Scope.js @@ -46,7 +46,7 @@ export default class Scope { if ( options.params ) { options.params.forEach( param => { extractNames( param ).forEach( name => { - this.declarations[ name ] = new Declaration(); + this.declarations[ name ] = new Declaration( param, true ); }); }); } diff --git a/src/utils/testForSideEffects.js b/src/utils/testForSideEffects.js index 1a42eba..c92622a 100644 --- a/src/utils/testForSideEffects.js +++ b/src/utils/testForSideEffects.js @@ -8,9 +8,7 @@ let pureFunctions = {}; [ // TODO add others to this list from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects 'Array', 'Array.isArray', - 'Error', 'EvalError', 'InternalError', 'RangeError', 'ReferenceError', 'SyntaxError', 'TypeError', 'URIError', - 'Object', 'Object.keys' ].forEach( name => pureFunctions[ name ] = true ); @@ -51,8 +49,6 @@ export default function testForSideEffects ( node, scope, statement, strongDepen } else if ( !pureFunctions[ node.callee.name ] ) { hasSideEffect = true; } - - // TODO does function mutate inputs that are needed? } else if ( node.callee.type === 'MemberExpression' ) { @@ -83,8 +79,12 @@ export default function testForSideEffects ( node, scope, statement, strongDepen let subject = node[ modifierNodes[ node.type ] ]; while ( subject.type === 'MemberExpression' ) subject = subject.object; - if ( !scope.findDeclaration( subject.name ) ) { - const declaration = statement.module.trace( subject.name ); + let declaration = scope.findDeclaration( subject.name ); + + if ( declaration ) { + hasSideEffect = declaration.isParam; + } else { + declaration = statement.module.trace( subject.name ); if ( !declaration || declaration.isExternal || declaration.isUsed ) { hasSideEffect = true; From 2dea1ef0e54c5184489c1c530e478f2755d50f6c Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 31 Oct 2015 23:39:02 -0400 Subject: [PATCH 16/29] add aggressive mode --- src/Bundle.js | 7 ++++++- test/form/aggressive/_config.js | 6 ++++++ test/form/aggressive/_expected/amd.js | 9 +++++++++ test/form/aggressive/_expected/cjs.js | 7 +++++++ test/form/aggressive/_expected/es6.js | 5 +++++ test/form/aggressive/_expected/iife.js | 9 +++++++++ test/form/aggressive/_expected/umd.js | 13 +++++++++++++ test/form/aggressive/foo.js | 9 +++++++++ test/form/aggressive/main.js | 3 +++ 9 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 test/form/aggressive/_config.js create mode 100644 test/form/aggressive/_expected/amd.js create mode 100644 test/form/aggressive/_expected/cjs.js create mode 100644 test/form/aggressive/_expected/es6.js create mode 100644 test/form/aggressive/_expected/iife.js create mode 100644 test/form/aggressive/_expected/umd.js create mode 100644 test/form/aggressive/foo.js create mode 100644 test/form/aggressive/main.js diff --git a/src/Bundle.js b/src/Bundle.js index 4130909..0947286 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -50,6 +50,7 @@ export default class Bundle { this.external = options.external || []; this.onwarn = options.onwarn || onwarn; + this.aggressive = options.aggressive; // TODO strictly speaking, this only applies with non-ES6, non-default-only bundles [ 'module', 'exports' ].forEach( global => this.assumedGlobals[ global ] = true ); @@ -74,7 +75,11 @@ export default class Bundle { }); // mark statements that should appear in the bundle - this.modules.forEach( module => module.markStatements() ); + if ( this.aggressive ) { + entryModule.markStatements(); + } else { + this.modules.forEach( module => module.markStatements() ); + } this.orderedModules = this.sort(); this.deconflict(); diff --git a/test/form/aggressive/_config.js b/test/form/aggressive/_config.js new file mode 100644 index 0000000..bafb1e0 --- /dev/null +++ b/test/form/aggressive/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: 'ignores side-effects outside entry module in aggressive mode', + options: { + aggressive: true + } +} diff --git a/test/form/aggressive/_expected/amd.js b/test/form/aggressive/_expected/amd.js new file mode 100644 index 0000000..eac4f98 --- /dev/null +++ b/test/form/aggressive/_expected/amd.js @@ -0,0 +1,9 @@ +define(function () { 'use strict'; + + function foo () { + return 42; + } + + assert.equal( foo(), 42 ); + +}); \ No newline at end of file diff --git a/test/form/aggressive/_expected/cjs.js b/test/form/aggressive/_expected/cjs.js new file mode 100644 index 0000000..a547fb6 --- /dev/null +++ b/test/form/aggressive/_expected/cjs.js @@ -0,0 +1,7 @@ +'use strict'; + +function foo () { + return 42; +} + +assert.equal( foo(), 42 ); \ No newline at end of file diff --git a/test/form/aggressive/_expected/es6.js b/test/form/aggressive/_expected/es6.js new file mode 100644 index 0000000..f32ad00 --- /dev/null +++ b/test/form/aggressive/_expected/es6.js @@ -0,0 +1,5 @@ +function foo () { + return 42; +} + +assert.equal( foo(), 42 ); \ No newline at end of file diff --git a/test/form/aggressive/_expected/iife.js b/test/form/aggressive/_expected/iife.js new file mode 100644 index 0000000..3ba1058 --- /dev/null +++ b/test/form/aggressive/_expected/iife.js @@ -0,0 +1,9 @@ +(function () { 'use strict'; + + function foo () { + return 42; + } + + assert.equal( foo(), 42 ); + +})(); \ No newline at end of file diff --git a/test/form/aggressive/_expected/umd.js b/test/form/aggressive/_expected/umd.js new file mode 100644 index 0000000..1eb1ef2 --- /dev/null +++ b/test/form/aggressive/_expected/umd.js @@ -0,0 +1,13 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? factory() : + typeof define === 'function' && define.amd ? define(factory) : + factory(); +}(this, function () { 'use strict'; + + function foo () { + return 42; + } + + assert.equal( foo(), 42 ); + +})); \ No newline at end of file diff --git a/test/form/aggressive/foo.js b/test/form/aggressive/foo.js new file mode 100644 index 0000000..16f57e9 --- /dev/null +++ b/test/form/aggressive/foo.js @@ -0,0 +1,9 @@ +function x () { + console.log( 'side-effect' ); +} + +x(); + +export function foo () { + return 42; +} diff --git a/test/form/aggressive/main.js b/test/form/aggressive/main.js new file mode 100644 index 0000000..66c6979 --- /dev/null +++ b/test/form/aggressive/main.js @@ -0,0 +1,3 @@ +import { foo } from './foo'; + +assert.equal( foo(), 42 ); From 0ffe08b1ffd034cf5337544280f08b7e1ef0e1b9 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 00:01:11 -0400 Subject: [PATCH 17/29] skip throw statements inside functions when checking for side-effects --- src/utils/testForSideEffects.js | 4 +++- test/form/side-effect-h/_config.js | 6 ++++++ test/form/side-effect-h/_expected/amd.js | 7 +++++++ test/form/side-effect-h/_expected/cjs.js | 5 +++++ test/form/side-effect-h/_expected/es6.js | 3 +++ test/form/side-effect-h/_expected/iife.js | 7 +++++++ test/form/side-effect-h/_expected/umd.js | 11 +++++++++++ test/form/side-effect-h/main.js | 9 +++++++++ 8 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 test/form/side-effect-h/_config.js create mode 100644 test/form/side-effect-h/_expected/amd.js create mode 100644 test/form/side-effect-h/_expected/cjs.js create mode 100644 test/form/side-effect-h/_expected/es6.js create mode 100644 test/form/side-effect-h/_expected/iife.js create mode 100644 test/form/side-effect-h/_expected/umd.js create mode 100644 test/form/side-effect-h/main.js diff --git a/src/utils/testForSideEffects.js b/src/utils/testForSideEffects.js index c92622a..076a35d 100644 --- a/src/utils/testForSideEffects.js +++ b/src/utils/testForSideEffects.js @@ -34,7 +34,9 @@ export default function testForSideEffects ( node, scope, statement, strongDepen } else if ( node.type === 'ThrowStatement' ) { - hasSideEffect = true; + // we only care about errors thrown at the top level, otherwise + // any function with error checking gets included if called + hasSideEffect = !scope.parent; } else if ( node.type === 'CallExpression' || node.type === 'NewExpression' ) { diff --git a/test/form/side-effect-h/_config.js b/test/form/side-effect-h/_config.js new file mode 100644 index 0000000..835691b --- /dev/null +++ b/test/form/side-effect-h/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: 'excludes non-top-level throw statements', + options: { + moduleName: 'myBundle' + } +}; diff --git a/test/form/side-effect-h/_expected/amd.js b/test/form/side-effect-h/_expected/amd.js new file mode 100644 index 0000000..37d2571 --- /dev/null +++ b/test/form/side-effect-h/_expected/amd.js @@ -0,0 +1,7 @@ +define(function () { 'use strict'; + + var main = 42; + + return main; + +}); diff --git a/test/form/side-effect-h/_expected/cjs.js b/test/form/side-effect-h/_expected/cjs.js new file mode 100644 index 0000000..5a370cd --- /dev/null +++ b/test/form/side-effect-h/_expected/cjs.js @@ -0,0 +1,5 @@ +'use strict'; + +var main = 42; + +module.exports = main; diff --git a/test/form/side-effect-h/_expected/es6.js b/test/form/side-effect-h/_expected/es6.js new file mode 100644 index 0000000..d862de8 --- /dev/null +++ b/test/form/side-effect-h/_expected/es6.js @@ -0,0 +1,3 @@ +var main = 42; + +export default main; diff --git a/test/form/side-effect-h/_expected/iife.js b/test/form/side-effect-h/_expected/iife.js new file mode 100644 index 0000000..b7b15ee --- /dev/null +++ b/test/form/side-effect-h/_expected/iife.js @@ -0,0 +1,7 @@ +var myBundle = (function () { 'use strict'; + + var main = 42; + + return main; + +})(); diff --git a/test/form/side-effect-h/_expected/umd.js b/test/form/side-effect-h/_expected/umd.js new file mode 100644 index 0000000..b5aa08a --- /dev/null +++ b/test/form/side-effect-h/_expected/umd.js @@ -0,0 +1,11 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() : + typeof define === 'function' && define.amd ? define(factory) : + global.myBundle = factory(); +}(this, function () { 'use strict'; + + var main = 42; + + return main; + +})); diff --git a/test/form/side-effect-h/main.js b/test/form/side-effect-h/main.js new file mode 100644 index 0000000..16d7346 --- /dev/null +++ b/test/form/side-effect-h/main.js @@ -0,0 +1,9 @@ +function foo ( ok ) { + if ( !ok ) { + throw new Error( 'this will be ignored' ); + } +} + +foo(); + +export default 42; From 18819ac61f7fbff472bb3b69b1321f529d8ab47b Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 09:23:47 -0500 Subject: [PATCH 18/29] memoize Declaration.testForSideEffects, prevent infinite loops --- src/Declaration.js | 13 ++++++++++--- test/function/conditional-definition/_config.js | 3 +++ test/function/conditional-definition/env.js | 13 +++++++++++++ test/function/conditional-definition/main.js | 4 ++++ 4 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 test/function/conditional-definition/_config.js create mode 100644 test/function/conditional-definition/env.js create mode 100644 test/function/conditional-definition/main.js diff --git a/src/Declaration.js b/src/Declaration.js index 70f94a8..74c39c2 100644 --- a/src/Declaration.js +++ b/src/Declaration.js @@ -33,9 +33,16 @@ export default class Declaration { } testForSideEffects ( strongDependencies ) { - // TODO memoize this - if ( !this.statement || !this.functionNode ) return; - return testForSideEffects( this.functionNode.body, this.functionNode._scope, this.statement, strongDependencies ); + if ( this.tested ) return this.hasSideEffects; + this.tested = true; + + if ( !this.statement || !this.functionNode ) { + this.hasSideEffects = false; + } else { + this.hasSideEffects = testForSideEffects( this.functionNode.body, this.functionNode._scope, this.statement, strongDependencies ); + } + + return this.hasSideEffects; } render ( es6 ) { diff --git a/test/function/conditional-definition/_config.js b/test/function/conditional-definition/_config.js new file mode 100644 index 0000000..a6b22f4 --- /dev/null +++ b/test/function/conditional-definition/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'avoids infinite loop when testing conditional definitions' +}; diff --git a/test/function/conditional-definition/env.js b/test/function/conditional-definition/env.js new file mode 100644 index 0000000..d35efdb --- /dev/null +++ b/test/function/conditional-definition/env.js @@ -0,0 +1,13 @@ +let env; + +if ( typeof window !== 'undefined' ) { + env = function () { + return 'browser'; + }; +} else { + env = function () { + return 'node'; + }; +} + +export { env }; diff --git a/test/function/conditional-definition/main.js b/test/function/conditional-definition/main.js new file mode 100644 index 0000000..197d1f0 --- /dev/null +++ b/test/function/conditional-definition/main.js @@ -0,0 +1,4 @@ +import { env } from './env'; + +assert.equal( env(), 'node' ); +assert.equal( env(), 'node' ); // to check that the env Declaration is only tested once From 6c876bb110812e3fdecbf551b657b5dce22f0e5d Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 10:17:51 -0500 Subject: [PATCH 19/29] include reassignments that are ignored on initial pass --- src/Bundle.js | 17 +++++++++++++---- src/Module.js | 6 +++++- src/Statement.js | 3 ++- test/function/includes-reassignments/_config.js | 3 +++ test/function/includes-reassignments/foo.js | 9 +++++++++ test/function/includes-reassignments/main.js | 3 +++ 6 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 test/function/includes-reassignments/_config.js create mode 100644 test/function/includes-reassignments/foo.js create mode 100644 test/function/includes-reassignments/main.js diff --git a/src/Bundle.js b/src/Bundle.js index 0947286..c2b6cd8 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -75,10 +75,19 @@ export default class Bundle { }); // mark statements that should appear in the bundle - if ( this.aggressive ) { - entryModule.markStatements(); - } else { - this.modules.forEach( module => module.markStatements() ); + let settled = false; + while ( !settled ) { + settled = true; + + if ( this.aggressive ) { + settled = !entryModule.markStatements(); + } else { + this.modules.forEach( module => { + if ( module.markStatements() ) { + settled = false; + } + }); + } } this.orderedModules = this.sort(); diff --git a/src/Module.js b/src/Module.js index b777ea6..3d1211d 100644 --- a/src/Module.js +++ b/src/Module.js @@ -279,9 +279,13 @@ export default class Module { } markStatements () { + let marked = false; + this.statements.forEach( statement => { - statement.secondPass( this.strongDependencies ); + marked = marked || statement.secondPass( this.strongDependencies ); }); + + return marked; } namespace () { diff --git a/src/Statement.js b/src/Statement.js index 00e0fcc..c76df95 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -157,7 +157,8 @@ export default class Statement { } secondPass ( strongDependencies ) { - if ( this.isImportDeclaration || this.isFunctionDeclaration ) return; + if ( ( this.tested && this.isIncluded ) || this.isImportDeclaration || this.isFunctionDeclaration ) return; + this.tested = true; if ( testForSideEffects( this.node, this.scope, this, strongDependencies ) ) { this.mark(); diff --git a/test/function/includes-reassignments/_config.js b/test/function/includes-reassignments/_config.js new file mode 100644 index 0000000..1d66654 --- /dev/null +++ b/test/function/includes-reassignments/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'includes reassignments that are ignored on an initial pass' +}; diff --git a/test/function/includes-reassignments/foo.js b/test/function/includes-reassignments/foo.js new file mode 100644 index 0000000..85beb3f --- /dev/null +++ b/test/function/includes-reassignments/foo.js @@ -0,0 +1,9 @@ +var answer, getAnswer; + +getAnswer = function () { + return 42; +}; + +answer = getAnswer(); + +export default answer; diff --git a/test/function/includes-reassignments/main.js b/test/function/includes-reassignments/main.js new file mode 100644 index 0000000..6236140 --- /dev/null +++ b/test/function/includes-reassignments/main.js @@ -0,0 +1,3 @@ +import answer from './foo.js'; + +assert.equal( answer, 42 ); From 809acd29ab572376935bf98044209a9025fe18ac Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 10:30:27 -0500 Subject: [PATCH 20/29] include top-level throw statement inside block --- src/ast/Scope.js | 1 + src/utils/testForSideEffects.js | 2 +- test/form/side-effect-i/_config.js | 6 ++++++ test/form/side-effect-i/_expected/amd.js | 11 +++++++++++ test/form/side-effect-i/_expected/cjs.js | 9 +++++++++ test/form/side-effect-i/_expected/es6.js | 7 +++++++ test/form/side-effect-i/_expected/iife.js | 11 +++++++++++ test/form/side-effect-i/_expected/umd.js | 15 +++++++++++++++ test/form/side-effect-i/main.js | 5 +++++ 9 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 test/form/side-effect-i/_config.js create mode 100644 test/form/side-effect-i/_expected/amd.js create mode 100644 test/form/side-effect-i/_expected/cjs.js create mode 100644 test/form/side-effect-i/_expected/es6.js create mode 100644 test/form/side-effect-i/_expected/iife.js create mode 100644 test/form/side-effect-i/_expected/umd.js create mode 100644 test/form/side-effect-i/main.js diff --git a/src/ast/Scope.js b/src/ast/Scope.js index 3878c8b..1adcd47 100644 --- a/src/ast/Scope.js +++ b/src/ast/Scope.js @@ -40,6 +40,7 @@ export default class Scope { this.parent = options.parent; this.isBlockScope = !!options.block; + this.isTopLevel = !this.parent || ( this.parent.isTopLevel && this.isBlockScope ); this.declarations = blank(); diff --git a/src/utils/testForSideEffects.js b/src/utils/testForSideEffects.js index 076a35d..04a4cd7 100644 --- a/src/utils/testForSideEffects.js +++ b/src/utils/testForSideEffects.js @@ -36,7 +36,7 @@ export default function testForSideEffects ( node, scope, statement, strongDepen else if ( node.type === 'ThrowStatement' ) { // we only care about errors thrown at the top level, otherwise // any function with error checking gets included if called - hasSideEffect = !scope.parent; + hasSideEffect = scope.isTopLevel; } else if ( node.type === 'CallExpression' || node.type === 'NewExpression' ) { diff --git a/test/form/side-effect-i/_config.js b/test/form/side-effect-i/_config.js new file mode 100644 index 0000000..ff58802 --- /dev/null +++ b/test/form/side-effect-i/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: 'includes top-level throw statements', + options: { + moduleName: 'myBundle' + } +}; diff --git a/test/form/side-effect-i/_expected/amd.js b/test/form/side-effect-i/_expected/amd.js new file mode 100644 index 0000000..21c57f9 --- /dev/null +++ b/test/form/side-effect-i/_expected/amd.js @@ -0,0 +1,11 @@ +define(function () { 'use strict'; + + if ( !ok ) { + throw new Error( 'this will be included' ); + } + + var main = 42; + + return main; + +}); diff --git a/test/form/side-effect-i/_expected/cjs.js b/test/form/side-effect-i/_expected/cjs.js new file mode 100644 index 0000000..fe9b519 --- /dev/null +++ b/test/form/side-effect-i/_expected/cjs.js @@ -0,0 +1,9 @@ +'use strict'; + +if ( !ok ) { + throw new Error( 'this will be included' ); +} + +var main = 42; + +module.exports = main; diff --git a/test/form/side-effect-i/_expected/es6.js b/test/form/side-effect-i/_expected/es6.js new file mode 100644 index 0000000..32ac89e --- /dev/null +++ b/test/form/side-effect-i/_expected/es6.js @@ -0,0 +1,7 @@ +if ( !ok ) { + throw new Error( 'this will be included' ); +} + +var main = 42; + +export default main; diff --git a/test/form/side-effect-i/_expected/iife.js b/test/form/side-effect-i/_expected/iife.js new file mode 100644 index 0000000..b728697 --- /dev/null +++ b/test/form/side-effect-i/_expected/iife.js @@ -0,0 +1,11 @@ +var myBundle = (function () { 'use strict'; + + if ( !ok ) { + throw new Error( 'this will be included' ); + } + + var main = 42; + + return main; + +})(); diff --git a/test/form/side-effect-i/_expected/umd.js b/test/form/side-effect-i/_expected/umd.js new file mode 100644 index 0000000..767e79a --- /dev/null +++ b/test/form/side-effect-i/_expected/umd.js @@ -0,0 +1,15 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() : + typeof define === 'function' && define.amd ? define(factory) : + global.myBundle = factory(); +}(this, function () { 'use strict'; + + if ( !ok ) { + throw new Error( 'this will be included' ); + } + + var main = 42; + + return main; + +})); diff --git a/test/form/side-effect-i/main.js b/test/form/side-effect-i/main.js new file mode 100644 index 0000000..ed0ee3c --- /dev/null +++ b/test/form/side-effect-i/main.js @@ -0,0 +1,5 @@ +if ( !ok ) { + throw new Error( 'this will be included' ); +} + +export default 42; From f1ad31c8ba09be38d0ff3f02328d14c1677e784f Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 10:48:55 -0500 Subject: [PATCH 21/29] err on side of caution with late definitions --- src/Declaration.js | 2 +- test/form/side-effect-e/_config.js | 2 +- test/form/side-effect-j/_config.js | 6 ++++++ test/form/side-effect-j/_expected/amd.js | 5 +++++ test/form/side-effect-j/_expected/cjs.js | 1 + test/form/side-effect-j/_expected/es6.js | 0 test/form/side-effect-j/_expected/iife.js | 0 test/form/side-effect-j/_expected/umd.js | 0 test/form/side-effect-j/foo.js | 4 ++++ test/form/side-effect-j/main.js | 4 ++++ 10 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 test/form/side-effect-j/_config.js create mode 100644 test/form/side-effect-j/_expected/amd.js create mode 100644 test/form/side-effect-j/_expected/cjs.js create mode 100644 test/form/side-effect-j/_expected/es6.js create mode 100644 test/form/side-effect-j/_expected/iife.js create mode 100644 test/form/side-effect-j/_expected/umd.js create mode 100644 test/form/side-effect-j/foo.js create mode 100644 test/form/side-effect-j/main.js diff --git a/src/Declaration.js b/src/Declaration.js index 74c39c2..abe86b5 100644 --- a/src/Declaration.js +++ b/src/Declaration.js @@ -37,7 +37,7 @@ export default class Declaration { this.tested = true; if ( !this.statement || !this.functionNode ) { - this.hasSideEffects = false; + this.hasSideEffects = true; // err on the side of caution. TODO handle unambiguous `var x; x = y => z` cases } else { this.hasSideEffects = testForSideEffects( this.functionNode.body, this.functionNode._scope, this.statement, strongDependencies ); } diff --git a/test/form/side-effect-e/_config.js b/test/form/side-effect-e/_config.js index a8d5bf9..225bacd 100644 --- a/test/form/side-effect-e/_config.js +++ b/test/form/side-effect-e/_config.js @@ -1,5 +1,5 @@ module.exports = { - description: 'excludes functions that are known to be pure', + description: 'accounts for local scopes when tested function purity', options: { moduleName: 'myBundle' } diff --git a/test/form/side-effect-j/_config.js b/test/form/side-effect-j/_config.js new file mode 100644 index 0000000..91550c0 --- /dev/null +++ b/test/form/side-effect-j/_config.js @@ -0,0 +1,6 @@ +module.exports = { + description: 'includes late function declarations with side-effects', + options: { + moduleName: 'myBundle' + } +}; diff --git a/test/form/side-effect-j/_expected/amd.js b/test/form/side-effect-j/_expected/amd.js new file mode 100644 index 0000000..85bce30 --- /dev/null +++ b/test/form/side-effect-j/_expected/amd.js @@ -0,0 +1,5 @@ +define(function () { 'use strict'; + + + +}); diff --git a/test/form/side-effect-j/_expected/cjs.js b/test/form/side-effect-j/_expected/cjs.js new file mode 100644 index 0000000..ad9a93a --- /dev/null +++ b/test/form/side-effect-j/_expected/cjs.js @@ -0,0 +1 @@ +'use strict'; diff --git a/test/form/side-effect-j/_expected/es6.js b/test/form/side-effect-j/_expected/es6.js new file mode 100644 index 0000000..e69de29 diff --git a/test/form/side-effect-j/_expected/iife.js b/test/form/side-effect-j/_expected/iife.js new file mode 100644 index 0000000..e69de29 diff --git a/test/form/side-effect-j/_expected/umd.js b/test/form/side-effect-j/_expected/umd.js new file mode 100644 index 0000000..e69de29 diff --git a/test/form/side-effect-j/foo.js b/test/form/side-effect-j/foo.js new file mode 100644 index 0000000..2f4073f --- /dev/null +++ b/test/form/side-effect-j/foo.js @@ -0,0 +1,4 @@ +var augment; +augment = x => x.augmented = true; + +export { augment }; diff --git a/test/form/side-effect-j/main.js b/test/form/side-effect-j/main.js new file mode 100644 index 0000000..3032b85 --- /dev/null +++ b/test/form/side-effect-j/main.js @@ -0,0 +1,4 @@ +import { augment } from './foo.js'; + +export default function x () {} +augment( x ); From df9e67eec9dcd1d9af63f42f2925b1c4374b2a38 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 11:15:49 -0500 Subject: [PATCH 22/29] use of arguments is assumed to create side-effects --- src/utils/testForSideEffects.js | 4 +++- test/form/side-effect-k/_config.js | 8 ++++++++ test/form/side-effect-k/_expected/amd.js | 5 +++++ test/form/side-effect-k/_expected/cjs.js | 1 + test/form/side-effect-k/_expected/es6.js | 0 test/form/side-effect-k/_expected/iife.js | 0 test/form/side-effect-k/_expected/umd.js | 0 test/form/side-effect-k/foo.js | 19 +++++++++++++++++++ test/form/side-effect-k/main.js | 4 ++++ 9 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 test/form/side-effect-k/_config.js create mode 100644 test/form/side-effect-k/_expected/amd.js create mode 100644 test/form/side-effect-k/_expected/cjs.js create mode 100644 test/form/side-effect-k/_expected/es6.js create mode 100644 test/form/side-effect-k/_expected/iife.js create mode 100644 test/form/side-effect-k/_expected/umd.js create mode 100644 test/form/side-effect-k/foo.js create mode 100644 test/form/side-effect-k/main.js diff --git a/src/utils/testForSideEffects.js b/src/utils/testForSideEffects.js index 04a4cd7..a6219f3 100644 --- a/src/utils/testForSideEffects.js +++ b/src/utils/testForSideEffects.js @@ -24,7 +24,9 @@ export default function testForSideEffects ( node, scope, statement, strongDepen if ( isReference( node, parent ) ) { const flattened = flatten( node ); - if ( !scope.contains( flattened.name ) ) { + if ( flattened.name === 'arguments' ) { + hasSideEffect = true; + } if ( !scope.contains( flattened.name ) ) { const declaration = statement.module.trace( flattened.name ); if ( declaration && !declaration.isExternal ) { const module = declaration.module || declaration.statement.module; // TODO is this right? diff --git a/test/form/side-effect-k/_config.js b/test/form/side-effect-k/_config.js new file mode 100644 index 0000000..093b570 --- /dev/null +++ b/test/form/side-effect-k/_config.js @@ -0,0 +1,8 @@ +module.exports = { + solo: true, + show: true, + description: 'use of arguments is treated as a side-effect', + options: { + moduleName: 'myBundle' + } +}; diff --git a/test/form/side-effect-k/_expected/amd.js b/test/form/side-effect-k/_expected/amd.js new file mode 100644 index 0000000..85bce30 --- /dev/null +++ b/test/form/side-effect-k/_expected/amd.js @@ -0,0 +1,5 @@ +define(function () { 'use strict'; + + + +}); diff --git a/test/form/side-effect-k/_expected/cjs.js b/test/form/side-effect-k/_expected/cjs.js new file mode 100644 index 0000000..ad9a93a --- /dev/null +++ b/test/form/side-effect-k/_expected/cjs.js @@ -0,0 +1 @@ +'use strict'; diff --git a/test/form/side-effect-k/_expected/es6.js b/test/form/side-effect-k/_expected/es6.js new file mode 100644 index 0000000..e69de29 diff --git a/test/form/side-effect-k/_expected/iife.js b/test/form/side-effect-k/_expected/iife.js new file mode 100644 index 0000000..e69de29 diff --git a/test/form/side-effect-k/_expected/umd.js b/test/form/side-effect-k/_expected/umd.js new file mode 100644 index 0000000..e69de29 diff --git a/test/form/side-effect-k/foo.js b/test/form/side-effect-k/foo.js new file mode 100644 index 0000000..e60ac33 --- /dev/null +++ b/test/form/side-effect-k/foo.js @@ -0,0 +1,19 @@ +export function augment ( x ) { + var prop, source; + + var i = arguments.length; + var sources = Array( i - 1 ); + while ( i-- ) { + sources[i-1] = arguments[i]; + } + + while (source = sources.shift()) { + for (prop in source) { + if (hasOwn.call(source, prop)) { + x[prop] = source[prop]; + } + } + } + + return x; +} diff --git a/test/form/side-effect-k/main.js b/test/form/side-effect-k/main.js new file mode 100644 index 0000000..bab1d82 --- /dev/null +++ b/test/form/side-effect-k/main.js @@ -0,0 +1,4 @@ +import { augment } from './foo.js'; + +export default function x () {} +augment( x.prototype ); From dfcfb92929b2eeb28f3f80fe04d556b6bc528bff Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 11:54:40 -0500 Subject: [PATCH 23/29] dont undo hasSideEffects --- src/utils/testForSideEffects.js | 4 +-- test/form/side-effect-k/_expected/amd.js | 27 +++++++++++++++++-- test/form/side-effect-k/_expected/cjs.js | 25 ++++++++++++++++++ test/form/side-effect-k/_expected/es6.js | 24 +++++++++++++++++ test/form/side-effect-k/_expected/iife.js | 28 ++++++++++++++++++++ test/form/side-effect-k/_expected/umd.js | 32 +++++++++++++++++++++++ 6 files changed, 136 insertions(+), 4 deletions(-) diff --git a/src/utils/testForSideEffects.js b/src/utils/testForSideEffects.js index a6219f3..7867004 100644 --- a/src/utils/testForSideEffects.js +++ b/src/utils/testForSideEffects.js @@ -38,7 +38,7 @@ export default function testForSideEffects ( node, scope, statement, strongDepen else if ( node.type === 'ThrowStatement' ) { // we only care about errors thrown at the top level, otherwise // any function with error checking gets included if called - hasSideEffect = scope.isTopLevel; + if ( scope.isTopLevel ) hasSideEffect = true; } else if ( node.type === 'CallExpression' || node.type === 'NewExpression' ) { @@ -86,7 +86,7 @@ export default function testForSideEffects ( node, scope, statement, strongDepen let declaration = scope.findDeclaration( subject.name ); if ( declaration ) { - hasSideEffect = declaration.isParam; + if ( declaration.isParam ) hasSideEffect = true; } else { declaration = statement.module.trace( subject.name ); diff --git a/test/form/side-effect-k/_expected/amd.js b/test/form/side-effect-k/_expected/amd.js index 85bce30..e903a01 100644 --- a/test/form/side-effect-k/_expected/amd.js +++ b/test/form/side-effect-k/_expected/amd.js @@ -1,5 +1,28 @@ define(function () { 'use strict'; - + function augment ( x ) { + var prop, source; -}); + var i = arguments.length; + var sources = Array( i - 1 ); + while ( i-- ) { + sources[i-1] = arguments[i]; + } + + while (source = sources.shift()) { + for (prop in source) { + if (hasOwn.call(source, prop)) { + x[prop] = source[prop]; + } + } + } + + return x; + } + + function x () {} + augment( x.prototype ); + + return x; + +}); \ No newline at end of file diff --git a/test/form/side-effect-k/_expected/cjs.js b/test/form/side-effect-k/_expected/cjs.js index ad9a93a..7ee74b0 100644 --- a/test/form/side-effect-k/_expected/cjs.js +++ b/test/form/side-effect-k/_expected/cjs.js @@ -1 +1,26 @@ 'use strict'; + +function augment ( x ) { + var prop, source; + + var i = arguments.length; + var sources = Array( i - 1 ); + while ( i-- ) { + sources[i-1] = arguments[i]; + } + + while (source = sources.shift()) { + for (prop in source) { + if (hasOwn.call(source, prop)) { + x[prop] = source[prop]; + } + } + } + + return x; +} + +function x () {} +augment( x.prototype ); + +module.exports = x; \ No newline at end of file diff --git a/test/form/side-effect-k/_expected/es6.js b/test/form/side-effect-k/_expected/es6.js index e69de29..0cbda65 100644 --- a/test/form/side-effect-k/_expected/es6.js +++ b/test/form/side-effect-k/_expected/es6.js @@ -0,0 +1,24 @@ +function augment ( x ) { + var prop, source; + + var i = arguments.length; + var sources = Array( i - 1 ); + while ( i-- ) { + sources[i-1] = arguments[i]; + } + + while (source = sources.shift()) { + for (prop in source) { + if (hasOwn.call(source, prop)) { + x[prop] = source[prop]; + } + } + } + + return x; +} + +function x () {} +augment( x.prototype ); + +export default x; \ No newline at end of file diff --git a/test/form/side-effect-k/_expected/iife.js b/test/form/side-effect-k/_expected/iife.js index e69de29..2929269 100644 --- a/test/form/side-effect-k/_expected/iife.js +++ b/test/form/side-effect-k/_expected/iife.js @@ -0,0 +1,28 @@ +var myBundle = (function () { 'use strict'; + + function augment ( x ) { + var prop, source; + + var i = arguments.length; + var sources = Array( i - 1 ); + while ( i-- ) { + sources[i-1] = arguments[i]; + } + + while (source = sources.shift()) { + for (prop in source) { + if (hasOwn.call(source, prop)) { + x[prop] = source[prop]; + } + } + } + + return x; + } + + function x () {} + augment( x.prototype ); + + return x; + +})(); \ No newline at end of file diff --git a/test/form/side-effect-k/_expected/umd.js b/test/form/side-effect-k/_expected/umd.js index e69de29..45f170b 100644 --- a/test/form/side-effect-k/_expected/umd.js +++ b/test/form/side-effect-k/_expected/umd.js @@ -0,0 +1,32 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() : + typeof define === 'function' && define.amd ? define(factory) : + global.myBundle = factory(); +}(this, function () { 'use strict'; + + function augment ( x ) { + var prop, source; + + var i = arguments.length; + var sources = Array( i - 1 ); + while ( i-- ) { + sources[i-1] = arguments[i]; + } + + while (source = sources.shift()) { + for (prop in source) { + if (hasOwn.call(source, prop)) { + x[prop] = source[prop]; + } + } + } + + return x; + } + + function x () {} + augment( x.prototype ); + + return x; + +})); \ No newline at end of file From 4b8046ab6ebfddc446c2fdc572da4a68cc7bcadc Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 11:57:46 -0500 Subject: [PATCH 24/29] update test --- test/form/side-effect-j/_expected/amd.js | 10 ++++++++-- test/form/side-effect-j/_expected/cjs.js | 8 ++++++++ test/form/side-effect-j/_expected/es6.js | 7 +++++++ test/form/side-effect-j/_expected/iife.js | 11 +++++++++++ test/form/side-effect-j/_expected/umd.js | 15 +++++++++++++++ test/form/side-effect-k/_config.js | 2 -- 6 files changed, 49 insertions(+), 4 deletions(-) diff --git a/test/form/side-effect-j/_expected/amd.js b/test/form/side-effect-j/_expected/amd.js index 85bce30..d5adfc9 100644 --- a/test/form/side-effect-j/_expected/amd.js +++ b/test/form/side-effect-j/_expected/amd.js @@ -1,5 +1,11 @@ define(function () { 'use strict'; - + var augment; + augment = x => x.augmented = true; -}); + function x () {} + augment( x ); + + return x; + +}); \ No newline at end of file diff --git a/test/form/side-effect-j/_expected/cjs.js b/test/form/side-effect-j/_expected/cjs.js index ad9a93a..e00c6bd 100644 --- a/test/form/side-effect-j/_expected/cjs.js +++ b/test/form/side-effect-j/_expected/cjs.js @@ -1 +1,9 @@ 'use strict'; + +var augment; +augment = x => x.augmented = true; + +function x () {} +augment( x ); + +module.exports = x; \ No newline at end of file diff --git a/test/form/side-effect-j/_expected/es6.js b/test/form/side-effect-j/_expected/es6.js index e69de29..b01df9b 100644 --- a/test/form/side-effect-j/_expected/es6.js +++ b/test/form/side-effect-j/_expected/es6.js @@ -0,0 +1,7 @@ +var augment; +augment = x => x.augmented = true; + +function x () {} +augment( x ); + +export default x; \ No newline at end of file diff --git a/test/form/side-effect-j/_expected/iife.js b/test/form/side-effect-j/_expected/iife.js index e69de29..7ab44e8 100644 --- a/test/form/side-effect-j/_expected/iife.js +++ b/test/form/side-effect-j/_expected/iife.js @@ -0,0 +1,11 @@ +var myBundle = (function () { 'use strict'; + + var augment; + augment = x => x.augmented = true; + + function x () {} + augment( x ); + + return x; + +})(); \ No newline at end of file diff --git a/test/form/side-effect-j/_expected/umd.js b/test/form/side-effect-j/_expected/umd.js index e69de29..e6e853f 100644 --- a/test/form/side-effect-j/_expected/umd.js +++ b/test/form/side-effect-j/_expected/umd.js @@ -0,0 +1,15 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() : + typeof define === 'function' && define.amd ? define(factory) : + global.myBundle = factory(); +}(this, function () { 'use strict'; + + var augment; + augment = x => x.augmented = true; + + function x () {} + augment( x ); + + return x; + +})); \ No newline at end of file diff --git a/test/form/side-effect-k/_config.js b/test/form/side-effect-k/_config.js index 093b570..f94d35b 100644 --- a/test/form/side-effect-k/_config.js +++ b/test/form/side-effect-k/_config.js @@ -1,6 +1,4 @@ module.exports = { - solo: true, - show: true, description: 'use of arguments is treated as a side-effect', options: { moduleName: 'myBundle' From b7dbe46dfece481a9188fb95bc8f5f4764b60243 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 12:07:34 -0500 Subject: [PATCH 25/29] add some comments, rename some things --- src/Bundle.js | 22 ++++++++---- src/Declaration.js | 38 ++++++++++----------- src/Module.js | 20 +++++------ src/Statement.js | 11 +++--- src/utils/{testForSideEffects.js => run.js} | 6 ++-- 5 files changed, 53 insertions(+), 44 deletions(-) rename src/utils/{testForSideEffects.js => run.js} (92%) diff --git a/src/Bundle.js b/src/Bundle.js index c2b6cd8..86f5403 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -39,7 +39,6 @@ export default class Bundle { .map( plugin => plugin.transform ) .filter( Boolean ); - this.pending = blank(); this.moduleById = blank(); this.modules = []; @@ -57,15 +56,23 @@ export default class Bundle { } build () { + // Phase 1 – discovery. We load the entry module and find which + // modules it imports, and import those, until we have all + // of the entry module's dependencies return Promise.resolve( this.resolveId( this.entry, undefined ) ) .then( id => this.fetchModule( id, undefined ) ) .then( entryModule => { this.entryModule = entryModule; + // Phase 2 – binding. We link references to their declarations + // to generate a complete picture of the bundle this.modules.forEach( module => module.bindImportSpecifiers() ); this.modules.forEach( module => module.bindAliases() ); this.modules.forEach( module => module.bindReferences() ); + // Phase 3 – marking. We 'run' each statement to see which ones + // need to be included in the generated bundle + // mark all export statements entryModule.getExports().forEach( name => { const declaration = entryModule.traceExport( name ); @@ -80,16 +87,17 @@ export default class Bundle { settled = true; if ( this.aggressive ) { - settled = !entryModule.markStatements(); + settled = !entryModule.run(); } else { this.modules.forEach( module => { - if ( module.markStatements() ) { - settled = false; - } + if ( module.run() ) settled = false; }); } } + // Phase 4 – final preparation. We order the modules with an + // enhanced topological sort that accounts for cycles, then + // ensure that names are deconflicted throughout the bundle this.orderedModules = this.sort(); this.deconflict(); }); @@ -129,8 +137,8 @@ export default class Bundle { fetchModule ( id, importer ) { // short-circuit cycles - if ( this.pending[ id ] ) return null; - this.pending[ id ] = true; + if ( id in this.moduleById ) return null; + this.moduleById[ id ] = null; return Promise.resolve( this.load( id ) ) .catch( err => { diff --git a/src/Declaration.js b/src/Declaration.js index abe86b5..03a5b17 100644 --- a/src/Declaration.js +++ b/src/Declaration.js @@ -1,5 +1,5 @@ import { blank, keys } from './utils/object.js'; -import testForSideEffects from './utils/testForSideEffects.js'; +import run from './utils/run.js'; export default class Declaration { constructor ( node, isParam ) { @@ -32,26 +32,26 @@ export default class Declaration { if ( reference.isReassignment ) this.isReassigned = true; } - testForSideEffects ( strongDependencies ) { + render ( es6 ) { + if ( es6 ) return this.name; + if ( !this.isReassigned || !this.isExported ) return this.name; + + return `exports.${this.name}`; + } + + run ( strongDependencies ) { if ( this.tested ) return this.hasSideEffects; this.tested = true; if ( !this.statement || !this.functionNode ) { this.hasSideEffects = true; // err on the side of caution. TODO handle unambiguous `var x; x = y => z` cases } else { - this.hasSideEffects = testForSideEffects( this.functionNode.body, this.functionNode._scope, this.statement, strongDependencies ); + this.hasSideEffects = run( this.functionNode.body, this.functionNode._scope, this.statement, strongDependencies ); } return this.hasSideEffects; } - render ( es6 ) { - if ( es6 ) return this.name; - if ( !this.isReassigned || !this.isExported ) return this.name; - - return `exports.${this.name}`; - } - use () { this.isUsed = true; if ( this.statement ) this.statement.mark(); @@ -87,22 +87,22 @@ export class SyntheticDefaultDeclaration { this.original = declaration; } - testForSideEffects ( strongDependencies ) { + render () { + return !this.original || this.original.isReassigned ? + this.name : + this.original.render(); + } + + run ( strongDependencies ) { if ( this.original ) { - return this.original.testForSideEffects( strongDependencies ); + return this.original.run( strongDependencies ); } if ( /FunctionExpression/.test( this.node.declaration.type ) ) { - return testForSideEffects( this.node.declaration.body, this.statement.scope, this.statement, strongDependencies ); + return run( this.node.declaration.body, this.statement.scope, this.statement, strongDependencies ); } } - render () { - return !this.original || this.original.isReassigned ? - this.name : - this.original.render(); - } - use () { this.isUsed = true; this.statement.mark(); diff --git a/src/Module.js b/src/Module.js index 3d1211d..ab23b19 100644 --- a/src/Module.js +++ b/src/Module.js @@ -278,16 +278,6 @@ export default class Module { return keys( exports ); } - markStatements () { - let marked = false; - - this.statements.forEach( statement => { - marked = marked || statement.secondPass( this.strongDependencies ); - }); - - return marked; - } - namespace () { if ( !this.declarations['*'] ) { this.declarations['*'] = new SyntheticNamespaceDeclaration( this ); @@ -556,6 +546,16 @@ export default class Module { return magicString.trim(); } + run () { + let marked = false; + + this.statements.forEach( statement => { + marked = marked || statement.run( this.strongDependencies ); + }); + + return marked; + } + trace ( name ) { if ( name in this.declarations ) return this.declarations[ name ]; if ( name in this.imports ) { diff --git a/src/Statement.js b/src/Statement.js index c76df95..203b60e 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -5,7 +5,7 @@ import modifierNodes from './ast/modifierNodes.js'; import isFunctionDeclaration from './ast/isFunctionDeclaration.js'; import isReference from './ast/isReference.js'; import getLocation from './utils/getLocation.js'; -import testForSideEffects from './utils/testForSideEffects.js'; +import run from './utils/run.js'; class Reference { constructor ( node, scope, statement ) { @@ -45,6 +45,7 @@ export default class Statement { this.stringLiteralRanges = []; this.isIncluded = false; + this.ran = false; this.isImportDeclaration = node.type === 'ImportDeclaration'; this.isExportDeclaration = /^Export/.test( node.type ); @@ -156,11 +157,11 @@ export default class Statement { }); } - secondPass ( strongDependencies ) { - if ( ( this.tested && this.isIncluded ) || this.isImportDeclaration || this.isFunctionDeclaration ) return; - this.tested = true; + run ( strongDependencies ) { + if ( ( this.ran && this.isIncluded ) || this.isImportDeclaration || this.isFunctionDeclaration ) return; + this.ran = true; - if ( testForSideEffects( this.node, this.scope, this, strongDependencies ) ) { + if ( run( this.node, this.scope, this, strongDependencies ) ) { this.mark(); return true; } diff --git a/src/utils/testForSideEffects.js b/src/utils/run.js similarity index 92% rename from src/utils/testForSideEffects.js rename to src/utils/run.js index 7867004..fa735ad 100644 --- a/src/utils/testForSideEffects.js +++ b/src/utils/run.js @@ -12,7 +12,7 @@ let pureFunctions = {}; 'Object', 'Object.keys' ].forEach( name => pureFunctions[ name ] = true ); -export default function testForSideEffects ( node, scope, statement, strongDependencies, force ) { +export default function run ( node, scope, statement, strongDependencies, force ) { let hasSideEffect = false; walk( node, { @@ -47,7 +47,7 @@ export default function testForSideEffects ( node, scope, statement, strongDepen statement.module.trace( node.callee.name ); if ( declaration ) { - if ( declaration.isExternal || declaration.testForSideEffects( strongDependencies ) ) { + if ( declaration.isExternal || declaration.run( strongDependencies ) ) { hasSideEffect = true; } } else if ( !pureFunctions[ node.callee.name ] ) { @@ -74,7 +74,7 @@ export default function testForSideEffects ( node, scope, statement, strongDepen } // otherwise we're probably dealing with a function expression - else if ( testForSideEffects( node.callee, scope, statement, strongDependencies, true ) ) { + else if ( run( node.callee, scope, statement, strongDependencies, true ) ) { hasSideEffect = true; } } From b0ecf463a1caa091c989e853bff78af5701ae8af Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 12:13:40 -0500 Subject: [PATCH 26/29] ugh node 0.12 --- test/function/conditional-definition/env.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/function/conditional-definition/env.js b/test/function/conditional-definition/env.js index d35efdb..dd7c175 100644 --- a/test/function/conditional-definition/env.js +++ b/test/function/conditional-definition/env.js @@ -1,4 +1,4 @@ -let env; +var env; if ( typeof window !== 'undefined' ) { env = function () { From 80ae9fa7a98db1fc9db3a42e4419695cf8c39518 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 16:20:05 -0500 Subject: [PATCH 27/29] remove unnecessary reference.isImmediatelyUsed --- src/Module.js | 5 ++++- src/Statement.js | 5 ++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Module.js b/src/Module.js index ab23b19..afa8b95 100644 --- a/src/Module.js +++ b/src/Module.js @@ -180,8 +180,11 @@ export default class Module { if ( statement.node.type !== 'VariableDeclaration' ) return; + const init = statement.node.declarations[0].init; + if ( !init || init.type === 'FunctionExpression' ) return; + statement.references.forEach( reference => { - if ( reference.name === name || !reference.isImmediatelyUsed ) return; + if ( reference.name === name ) return; const otherDeclaration = this.trace( reference.name ); if ( otherDeclaration ) otherDeclaration.addAlias( declaration ); diff --git a/src/Statement.js b/src/Statement.js index 203b60e..73cdf3d 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -133,11 +133,10 @@ export default class Statement { scope; const reference = new Reference( node, referenceScope, statement ); - references.push( reference ); - - reference.isImmediatelyUsed = !readDepth; reference.isReassignment = isReassignment; + references.push( reference ); + this.skip(); // don't descend from `foo.bar.baz` into `foo.bar` } }, From 7b57720d1328274f72df828a1f7e54c8af3dee2d Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 16:31:42 -0500 Subject: [PATCH 28/29] simplify consolidateDependencies --- src/Bundle.js | 12 ++++-------- src/Module.js | 16 ++++++---------- src/utils/run.js | 2 +- 3 files changed, 11 insertions(+), 19 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index 86f5403..1d3255b 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -274,12 +274,10 @@ export default class Bundle { strongDeps[ module.id ] = []; stronglyDependsOn[ module.id ] = {}; - keys( strongDependencies ).forEach( id => { - const imported = strongDependencies[ id ]; - + strongDependencies.forEach( imported => { strongDeps[ module.id ].push( imported ); - if ( seen[ id ] ) { + if ( seen[ imported.id ] ) { // we need to prevent an infinite loop, and note that // we need to check for strong/weak dependency relationships hasCycles = true; @@ -289,10 +287,8 @@ export default class Bundle { visit( imported ); }); - keys( weakDependencies ).forEach( id => { - const imported = weakDependencies[ id ]; - - if ( seen[ id ] ) { + weakDependencies.forEach( imported => { + if ( seen[ imported.id ] ) { // we need to prevent an infinite loop, and note that // we need to check for strong/weak dependency relationships hasCycles = true; diff --git a/src/Module.js b/src/Module.js index afa8b95..db6b85f 100644 --- a/src/Module.js +++ b/src/Module.js @@ -238,25 +238,21 @@ export default class Module { } consolidateDependencies () { - let strongDependencies = blank(); - let weakDependencies = blank(); + let strongDependencies = []; + let weakDependencies = []; // treat all imports as weak dependencies this.dependencies.forEach( source => { const id = this.resolvedIds[ source ]; const dependency = this.bundle.moduleById[ id ]; - if ( !dependency.isExternal ) { - // TODO this is a weird data structure - weakDependencies[ dependency.id ] = dependency; + if ( !dependency.isExternal && !~weakDependencies.indexOf( dependency ) ) { + weakDependencies.push( dependency ); } }); - this.strongDependencies.forEach( module => { - if ( module !== this ) { - strongDependencies[ module.id ] = module; - } - }); + strongDependencies = this.strongDependencies + .filter( module => module !== this ); return { strongDependencies, weakDependencies }; } diff --git a/src/utils/run.js b/src/utils/run.js index fa735ad..8c5d56b 100644 --- a/src/utils/run.js +++ b/src/utils/run.js @@ -30,7 +30,7 @@ export default function run ( node, scope, statement, strongDependencies, force const declaration = statement.module.trace( flattened.name ); if ( declaration && !declaration.isExternal ) { const module = declaration.module || declaration.statement.module; // TODO is this right? - if ( !~strongDependencies.indexOf( module ) ) strongDependencies.push( module ); + if ( !module.isExternal && !~strongDependencies.indexOf( module ) ) strongDependencies.push( module ); } } } From 39e75fe856e25ff870a2d7795960808f4846de67 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 1 Nov 2015 17:02:25 -0500 Subject: [PATCH 29/29] add a load more pure functions --- src/utils/run.js | 43 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/utils/run.js b/src/utils/run.js index 8c5d56b..2ae6212 100644 --- a/src/utils/run.js +++ b/src/utils/run.js @@ -5,12 +5,47 @@ import isReference from '../ast/isReference.js'; import flatten from '../ast/flatten'; let pureFunctions = {}; + +const arrayTypes = 'Array Int8Array Uint8Array Uint8ClampedArray Int16Array Uint16Array Int32Array Uint32Array Float32Array Float64Array'.split( ' ' ); +const simdTypes = 'Int8x16 Int16x8 Int32x4 Float32x4 Float64x2'.split( ' ' ); +const simdMethods = 'abs add and bool check div equal extractLane fromFloat32x4 fromFloat32x4Bits fromFloat64x2 fromFloat64x2Bits fromInt16x8Bits fromInt32x4 fromInt32x4Bits fromInt8x16Bits greaterThan greaterThanOrEqual lessThan lessThanOrEqual load max maxNum min minNum mul neg not notEqual or reciprocalApproximation reciprocalSqrtApproximation replaceLane select selectBits shiftLeftByScalar shiftRightArithmeticByScalar shiftRightLogicalByScalar shuffle splat sqrt store sub swizzle xor'.split( ' ' ); +let allSimdMethods = []; +simdTypes.forEach( t => { + simdMethods.forEach( m => { + allSimdMethods.push( `SIMD.${t}.${m}` ); + }); +}); + [ - // TODO add others to this list from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects - 'Array', 'Array.isArray', + 'Array.isArray', 'Error', 'EvalError', 'InternalError', 'RangeError', 'ReferenceError', 'SyntaxError', 'TypeError', 'URIError', - 'Object', 'Object.keys' -].forEach( name => pureFunctions[ name ] = true ); + 'isFinite', 'isNaN', 'parseFloat', 'parseInt', 'decodeURI', 'decodeURIComponent', 'encodeURI', 'encodeURIComponent', 'escape', 'unescape', + 'Object', 'Object.create', 'Object.getNotifier', 'Object.getOwn', 'Object.getOwnPropertyDescriptor', 'Object.getOwnPropertyNames', 'Object.getOwnPropertySymbols', 'Object.getPrototypeOf', 'Object.is', 'Object.isExtensible', 'Object.isFrozen', 'Object.isSealed', 'Object.keys', + 'Function', 'Boolean', + 'Number', 'Number.isFinite', 'Number.isInteger', 'Number.isNaN', 'Number.isSafeInteger', 'Number.parseFloat', 'Number.parseInt', + 'Symbol', 'Symbol.for', 'Symbol.keyFor', + 'Math.abs', 'Math.acos', 'Math.acosh', 'Math.asin', 'Math.asinh', 'Math.atan', 'Math.atan2', 'Math.atanh', 'Math.cbrt', 'Math.ceil', 'Math.clz32', 'Math.cos', 'Math.cosh', 'Math.exp', 'Math.expm1', 'Math.floor', 'Math.fround', 'Math.hypot', 'Math.imul', 'Math.log', 'Math.log10', 'Math.log1p', 'Math.log2', 'Math.max', 'Math.min', 'Math.pow', 'Math.random', 'Math.round', 'Math.sign', 'Math.sin', 'Math.sinh', 'Math.sqrt', 'Math.tan', 'Math.tanh', 'Math.trunc', + 'Date', 'Date.UTC', 'Date.now', 'Date.parse', + 'String', 'String.fromCharCode', 'String.fromCodePoint', 'String.raw', + 'RegExp', + 'Map', 'Set', 'WeakMap', 'WeakSet', + 'ArrayBuffer', 'ArrayBuffer.isView', + 'DataView', + 'JSON.parse', 'JSON.stringify', + 'Promise', 'Promise.all', 'Promise.race', 'Promise.reject', 'Promise.resolve', + 'Intl.Collator', 'Intl.Collator.supportedLocalesOf', 'Intl.DateTimeFormat', 'Intl.DateTimeFormat.supportedLocalesOf', 'Intl.NumberFormat', 'Intl.NumberFormat.supportedLocalesOf' + + // TODO properties of e.g. window... +].concat( + arrayTypes, + arrayTypes.map( t => `${t}.from` ), + arrayTypes.map( t => `${t}.of` ), + simdTypes.map( t => `SIMD.${t}` ), + allSimdMethods +).forEach( name => pureFunctions[ name ] = true ); + // TODO add others to this list from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects + + export default function run ( node, scope, statement, strongDependencies, force ) { let hasSideEffect = false;