Browse Source

make aggressive mode a bit less aggressive

better-aggressive
Rich-Harris 9 years ago
parent
commit
76502ea677
  1. 12
      src/Bundle.js
  2. 10
      src/Declaration.js
  3. 4
      src/Module.js
  4. 4
      src/Statement.js
  5. 16
      src/utils/run.js
  6. 8
      test/function/aggressive-mode/_config.js
  7. 11
      test/function/aggressive-mode/foo.js
  8. 3
      test/function/aggressive-mode/main.js

12
src/Bundle.js

@ -82,17 +82,15 @@ export default class Bundle {
}); });
// mark statements that should appear in the bundle // mark statements that should appear in the bundle
const safe = !this.aggressive;
let settled = false; let settled = false;
while ( !settled ) { while ( !settled ) {
settled = true; settled = true;
if ( this.aggressive ) { this.modules.forEach( module => {
settled = !entryModule.run(); if ( module.run( safe || module === entryModule ) ) settled = false;
} else { });
this.modules.forEach( module => {
if ( module.run() ) settled = false;
});
}
} }
// Phase 4 – final preparation. We order the modules with an // Phase 4 – final preparation. We order the modules with an

10
src/Declaration.js

@ -39,14 +39,14 @@ export default class Declaration {
return `exports.${this.name}`; return `exports.${this.name}`;
} }
run ( strongDependencies ) { run ( strongDependencies, safe ) {
if ( this.tested ) return this.hasSideEffects; if ( this.tested ) return this.hasSideEffects;
this.tested = true; this.tested = true;
if ( !this.statement || !this.functionNode ) { if ( !this.statement || !this.functionNode ) {
this.hasSideEffects = true; // err on the side of caution. TODO handle unambiguous `var x; x = y => z` cases this.hasSideEffects = true; // err on the side of caution. TODO handle unambiguous `var x; x = y => z` cases
} else { } else {
this.hasSideEffects = run( this.functionNode.body, this.functionNode._scope, this.statement, strongDependencies ); this.hasSideEffects = run( this.functionNode.body, this.functionNode._scope, this.statement, strongDependencies, false, safe );
} }
return this.hasSideEffects; return this.hasSideEffects;
@ -93,13 +93,13 @@ export class SyntheticDefaultDeclaration {
this.original.render(); this.original.render();
} }
run ( strongDependencies ) { run ( strongDependencies, safe ) {
if ( this.original ) { if ( this.original ) {
return this.original.run( strongDependencies ); return this.original.run( strongDependencies, safe );
} }
if ( /FunctionExpression/.test( this.node.declaration.type ) ) { if ( /FunctionExpression/.test( this.node.declaration.type ) ) {
return run( this.node.declaration.body, this.statement.scope, this.statement, strongDependencies ); return run( this.node.declaration.body, this.statement.scope, this.statement, strongDependencies, false, safe );
} }
} }

4
src/Module.js

@ -559,11 +559,11 @@ export default class Module {
return magicString.trim(); return magicString.trim();
} }
run () { run ( safe ) {
let marked = false; let marked = false;
this.statements.forEach( statement => { this.statements.forEach( statement => {
marked = marked || statement.run( this.strongDependencies ); marked = marked || statement.run( this.strongDependencies, safe );
}); });
return marked; return marked;

4
src/Statement.js

@ -159,11 +159,11 @@ export default class Statement {
}); });
} }
run ( strongDependencies ) { run ( strongDependencies, safe ) {
if ( ( this.ran && this.isIncluded ) || this.isImportDeclaration || this.isFunctionDeclaration ) return; if ( ( this.ran && this.isIncluded ) || this.isImportDeclaration || this.isFunctionDeclaration ) return;
this.ran = true; this.ran = true;
if ( run( this.node, this.scope, this, strongDependencies ) ) { if ( run( this.node, this.scope, this, strongDependencies, false, safe ) ) {
this.mark(); this.mark();
return true; return true;
} }

16
src/utils/run.js

@ -46,7 +46,7 @@ simdTypes.forEach( t => {
export default function run ( node, scope, statement, strongDependencies, force ) { export default function run ( node, scope, statement, strongDependencies, force, safe ) {
let hasSideEffect = false; let hasSideEffect = false;
walk( node, { walk( node, {
@ -60,7 +60,9 @@ export default function run ( node, scope, statement, strongDependencies, force
if ( flattened.name === 'arguments' ) { if ( flattened.name === 'arguments' ) {
hasSideEffect = true; hasSideEffect = true;
} if ( !scope.contains( flattened.name ) ) { }
else if ( !scope.contains( flattened.name ) ) {
const declaration = statement.module.trace( flattened.name ); const declaration = statement.module.trace( flattened.name );
if ( declaration && !declaration.isExternal ) { if ( declaration && !declaration.isExternal ) {
const module = declaration.module || declaration.statement.module; // TODO is this right? const module = declaration.module || declaration.statement.module; // TODO is this right?
@ -81,10 +83,10 @@ export default function run ( node, scope, statement, strongDependencies, force
statement.module.trace( node.callee.name ); statement.module.trace( node.callee.name );
if ( declaration ) { if ( declaration ) {
if ( declaration.isExternal || declaration.run( strongDependencies ) ) { if ( declaration.isExternal || declaration.run( strongDependencies, safe ) ) {
hasSideEffect = true; hasSideEffect = true;
} }
} else if ( !pureFunctions[ node.callee.name ] ) { } else if ( safe && !pureFunctions[ node.callee.name ] ) {
hasSideEffect = true; hasSideEffect = true;
} }
} }
@ -97,7 +99,7 @@ export default function run ( node, scope, statement, strongDependencies, force
// TODO make pureFunctions configurable // TODO make pureFunctions configurable
const declaration = scope.findDeclaration( flattened.name ) || statement.module.trace( flattened.name ); const declaration = scope.findDeclaration( flattened.name ) || statement.module.trace( flattened.name );
if ( !!declaration || !pureFunctions[ flattened.keypath ] ) { if ( safe && ( !!declaration || !pureFunctions[ flattened.keypath ] ) ) {
hasSideEffect = true; hasSideEffect = true;
} }
} else { } else {
@ -108,7 +110,7 @@ export default function run ( node, scope, statement, strongDependencies, force
} }
// otherwise we're probably dealing with a function expression // otherwise we're probably dealing with a function expression
else if ( run( node.callee, scope, statement, strongDependencies, true ) ) { else if ( run( node.callee, scope, statement, strongDependencies, true, safe ) ) {
hasSideEffect = true; hasSideEffect = true;
} }
} }
@ -124,7 +126,7 @@ export default function run ( node, scope, statement, strongDependencies, force
} else { } else {
declaration = statement.module.trace( subject.name ); declaration = statement.module.trace( subject.name );
if ( !declaration || declaration.isExternal || declaration.isUsed ) { if ( ( safe && ( !declaration || declaration.isExternal ) ) || declaration && declaration.isUsed ) {
hasSideEffect = true; hasSideEffect = true;
} }
} }

8
test/function/aggressive-mode/_config.js

@ -0,0 +1,8 @@
module.exports = {
// solo: true,
// show: true,
description: 'aggressive mode distinguishes between necessary and probably-not-necessary side-effects',
options: {
aggressive: true
}
};

11
test/function/aggressive-mode/foo.js

@ -0,0 +1,11 @@
function Foo () {}
Foo.prototype = {
answer: function () {
return 42;
}
};
export default function foo () {
return new Foo();
}

3
test/function/aggressive-mode/main.js

@ -0,0 +1,3 @@
import foo from './foo';
assert.equal( foo().answer(), 42 );
Loading…
Cancel
Save