better sorting (sort at module level)

src/Bundle.js

@@ -34,6 +34,8 @@ export default class Bundle {
 		this.toExport = null;
 
 		this.modulePromises = blank();
+		this.modules = [];
+
 		this.statements = [];
 		this.externalModules = [];
 		this.internalNamespaceModules = [];
@@ -63,6 +65,8 @@ export default class Bundle {
 								bundle: this
 							});
 
+							this.modules.push( module );
+
 							return module;
 						});
 				}
@@ -110,7 +114,8 @@ export default class Bundle {
 			.then( statements => {
 				this.statements = statements;
 				this.deconflict();
-				this.sort();
+
+				this.orderedStatements = this.sort();
 			});
 	}
 
@@ -200,100 +205,95 @@ export default class Bundle {
 	}
 
 	sort () {
-		// TODO avoid this work whenever possible...
+		let seen = {};
+		let ordered = [];
+		let hasCycles;
 
-		let definitions = blank();
+		let strongDeps = {};
+		let stronglyDependsOn = {};
 
-		// gather definitions
-		this.statements.forEach( statement => {
-			keys( statement.defines ).forEach( name => {
-				const canonicalName = statement.module.getCanonicalName( name );
-				definitions[ canonicalName ] = statement;
-			});
-		});
+		function visit ( module ) {
+			seen[ module.id ] = true;
 
-		let strongDeps = blank();
-		let stronglyDependsOn = blank();
+			const { strongDependencies, weakDependencies } = module.consolidateDependencies();
 
-		this.statements.forEach( statement => {
-			const id = statement.id;
-			strongDeps[ id ] = [];
-			stronglyDependsOn[ id ] = {};
+			strongDeps[ module.id ] = [];
+			stronglyDependsOn[ module.id ] = {};
+
+			keys( strongDependencies ).forEach( id => {
+				const imported = strongDependencies[ id ];
 
-			keys( statement.stronglyDependsOn ).forEach( name => {
-				if ( statement.defines[ name ] ) return; // TODO seriously... need to fix this
-				const canonicalName = statement.module.getCanonicalName( name );
-				const definition = definitions[ canonicalName ];
+				strongDeps[ module.id ].push( imported );
 
-				if ( definition ) strongDeps[ statement.id ].push( definition );
+				if ( seen[ id ] ) {
+					// we need to prevent an infinite loop, and note that
+					// we need to check for strong/weak dependency relationships
+					hasCycles = true;
+					return;
+				}
+
+				visit( imported );
 			});
-		});
 
-		// add second (and third...) order strong dependencies
-		this.statements.forEach( statement => {
-			const id = statement.id;
+			keys( weakDependencies ).forEach( id => {
+				const imported = weakDependencies[ id ];
+
+				if ( seen[ id ] ) {
+					// we need to prevent an infinite loop, and note that
+					// we need to check for strong/weak dependency relationships
+					hasCycles = true;
+					return;
+				}
+
+				visit( imported );
+			});
 
 			// add second (and third...) order dependencies
 			function addStrongDependencies ( dependency ) {
-				if ( stronglyDependsOn[ id ][ dependency.id ] ) return;
+				if ( stronglyDependsOn[ module.id ][ dependency.id ] ) return;
 
-				stronglyDependsOn[ id ][ dependency.id ] = true;
+				stronglyDependsOn[ module.id ][ dependency.id ] = true;
 				strongDeps[ dependency.id ].forEach( addStrongDependencies );
 			}
 
-			strongDeps[ id ].forEach( addStrongDependencies );
-		});
+			strongDeps[ module.id ].forEach( addStrongDependencies );
+
+			ordered.push( module );
+		}
+
+		visit( this.entryModule );
 
-		// reinsert each statement, ensuring its strong dependencies appear first
-		let sorted = [];
-		let included = blank();
-		let highestIndex = blank();
-
-		function include ( statement ) {
-			if ( included[ statement.id ] ) return;
-			included[ statement.id ] = true;
-
-			let alreadyIncluded = false;
-
-			const unordered = statement.index < highestIndex[ statement.module.id ];
-			highestIndex[ statement.module.id ] = Math.max(
-				statement.index,
-				highestIndex[ statement.module.id ] || 0
-			);
-
-			if ( unordered ) {
-				const len = sorted.length;
-				let i = 0;
-
-				for ( i = 0; i < len; i += 1 ) {
-					// ensure that this statement appears above later statements
-					// from the same module - in rare situations (#34) they can
-					// become jumbled
-					const existing = sorted[i];
-					if ( existing.module === statement.module && existing.index > statement.index ) {
-						sorted.splice( i, 0, statement );
-						return
+		if ( hasCycles ) {
+			let unordered = ordered;
+			ordered = [];
+
+			// unordered is actually semi-ordered, as [ fewer dependencies ... more dependencies ]
+			unordered.forEach( module => {
+				// ensure strong dependencies of `module` that don't strongly depend on `module` go first
+				strongDeps[ module.id ].forEach( place );
+
+				function place ( dep ) {
+					if ( !stronglyDependsOn[ dep.id ][ module.id ] && !~ordered.indexOf( dep ) ) {
+						strongDeps[ dep.id ].forEach( place );
+						ordered.push( dep );
 					}
 				}
-			}
 
-			sorted.push( statement );
+				if ( !~ordered.indexOf( module ) ) {
+					ordered.push( module );
+				}
+			});
 		}
 
-		this.statements.forEach( statement => {
-			strongDeps[ statement.id ].forEach( includeStrongDependency );
+		let statements = [];
 
-			function includeStrongDependency ( dependency ) {
-				if ( !stronglyDependsOn[ dependency.id ][ statement.id ] && !included[ dependency.id ] ) {
-					strongDeps[ dependency.id ].forEach( includeStrongDependency );
-					include( dependency );
-				}
-			}
-
-			include( statement );
+		ordered.forEach( module => {
+			module.statements.forEach( statement => {
+				if ( statement.isIncluded ) statements.push( statement );
+			});
 		});
 
-		this.statements = sorted;
+		return statements;
 	}
 
 	generate ( options = {} ) {
@@ -343,7 +343,7 @@ export default class Bundle {
 		let previousIndex = -1;
 		let previousMargin = 0;
 
-		this.statements.forEach( statement => {
+		this.orderedStatements.forEach( statement => {
 			// skip `export { foo, bar, baz }`
 			if ( statement.node.type === 'ExportNamedDeclaration' && statement.node.specifiers.length ) {
 				return;

src/Module.js

@@ -234,6 +234,43 @@ export default class Module {
 		});
 	}
 
+	consolidateDependencies () {
+		let strongDependencies = blank();
+
+		this.statements.forEach( statement => {
+			if ( statement.isImportDeclaration && !statement.node.specifiers.length ) {
+				// include module for its side-effects
+				strongDependencies[ statement.module.id ] = statement.module; // TODO is this right? `statement.module` should be `this`, surely?
+			}
+
+			keys( statement.stronglyDependsOn ).forEach( name => {
+				if ( statement.defines[ name ] ) return;
+
+				const importDeclaration = this.imports[ name ];
+
+				if ( importDeclaration && importDeclaration.module && !importDeclaration.module.isExternal ) {
+					strongDependencies[ importDeclaration.module.id ] = importDeclaration.module;
+				}
+			});
+		});
+
+		let weakDependencies = blank();
+
+		this.statements.forEach( statement => {
+			keys( statement.dependsOn ).forEach( name => {
+				if ( statement.defines[ name ] ) return;
+
+				const importDeclaration = this.imports[ name ];
+
+				if ( importDeclaration && importDeclaration.module && !importDeclaration.module.isExternal ) {
+					weakDependencies[ importDeclaration.module.id ] = importDeclaration.module;
+				}
+			});
+		});
+
+		return { strongDependencies, weakDependencies };
+	}
+
 	findDeclaration ( localName ) {
 		const importDeclaration = this.imports[ localName ];
 

test/function/retains-sort-order-b/_config.js

@@ -0,0 +1,8 @@
+var assert = require( 'assert' );
+
+module.exports = {
+	description: 'sorts statements according to their original order within modules, part 2',
+	exports: function ( exports ) {
+		assert.equal( exports.answer, 42 );
+	}
+};

test/function/retains-sort-order-b/bar.js

@@ -0,0 +1,7 @@
+var bar = {};
+
+bar.apply = function ( object ) {
+	object.answer = 42;
+};
+
+export { bar };

test/function/retains-sort-order-b/foo.js

@@ -0,0 +1,12 @@
+import { bar } from './bar';
+
+var Foo = function () {
+	this.id = incr();
+};
+
+bar.apply( Foo.prototype );
+
+var count = 0;
+function incr () { return count++; };
+
+export { Foo };

test/function/retains-sort-order-b/main.js

@@ -0,0 +1,3 @@
+import { Foo } from './foo';
+
+export default new Foo();