add some comments, rename some things

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() ) {
+							if ( module.run() ) settled = false;
-								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;
+		if ( id in this.moduleById ) return null;
-		this.pending[ id ] = true;
+		this.moduleById[ id ] = null;
 
 		return Promise.resolve( this.load( id ) )
 			.catch( err => {

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();

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 ) {

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 ) {
+	run ( strongDependencies ) {
-		if ( ( this.tested && this.isIncluded ) || this.isImportDeclaration || this.isFunctionDeclaration ) return;
+		if ( ( this.ran && this.isIncluded ) || this.isImportDeclaration || this.isFunctionDeclaration ) return;
-		this.tested = true;
+		this.ran = true;
 
-		if ( testForSideEffects( this.node, this.scope, this, strongDependencies ) ) {
+		if ( run( this.node, this.scope, this, strongDependencies ) ) {
 			this.mark();
 			return true;
 		}

src/utils/testForSideEffects.js → 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;
 				}
 			}