Merge branch 'gh-36'

src/Bundle.js

@@ -110,6 +110,7 @@ export default class Bundle {
 			.then( statements => {
 				this.statements = statements;
 				this.deconflict();
+				this.sort();
 			});
 	}
 
@@ -198,6 +199,76 @@ export default class Bundle {
 		}
 	}
 
+	sort () {
+		// TODO avoid this work whenever possible...
+
+		let definitions = blank();
+
+		// gather definitions
+		this.statements.forEach( statement => {
+			keys( statement.defines ).forEach( name => {
+				const canonicalName = statement.module.getCanonicalName( name );
+				definitions[ canonicalName ] = statement;
+			});
+		});
+
+		let strongDeps = blank();
+		let stronglyDependsOn = blank();
+
+		this.statements.forEach( statement => {
+			const id = statement.id;
+			strongDeps[ id ] = [];
+			stronglyDependsOn[ 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 ];
+
+				if ( definition ) strongDeps[ statement.id ].push( definition );
+			});
+		});
+
+		// add second (and third...) order strong dependencies
+		this.statements.forEach( statement => {
+			const id = statement.id;
+
+			// add second (and third...) order dependencies
+			function addStrongDependencies ( dependency ) {
+				if ( stronglyDependsOn[ id ][ dependency.id ] ) return;
+
+				stronglyDependsOn[ id ][ dependency.id ] = true;
+				strongDeps[ dependency.id ].forEach( addStrongDependencies );
+			}
+
+			strongDeps[ id ].forEach( addStrongDependencies );
+		});
+
+		// reinsert each statement, ensuring its strong dependencies appear first
+		let sorted = [];
+		let included = blank();
+
+		this.statements.forEach( statement => {
+			strongDeps[ statement.id ].forEach( place );
+
+			function place ( dependency ) {
+				if ( !stronglyDependsOn[ dependency.id ][ statement.id ] && !included[ dependency.id ] ) {
+					strongDeps[ dependency.id ].forEach( place );
+					sorted.push( dependency );
+
+					included[ dependency.id ] = true;
+				}
+			}
+
+			if ( !included[ statement.id ] ) {
+				sorted.push( statement );
+				included[ statement.id ] = true;
+			}
+		});
+
+		this.statements = sorted;
+	}
+
 	generate ( options = {} ) {
 		let magicString = new MagicString.Bundle({ separator: '' });
 

src/Statement.js

@@ -4,19 +4,19 @@ import getLocation from './utils/getLocation';
 import walk from './ast/walk';
 import Scope from './ast/Scope';
 
-const emptyArrayPromise = Promise.resolve([]);
-
 export default class Statement {
 	constructor ( node, magicString, module, index ) {
 		this.node = node;
 		this.module = module;
 		this.magicString = magicString;
 		this.index = index;
+		this.id = module.path + '#' + index;
 
 		this.scope = new Scope();
 		this.defines = blank();
 		this.modifies = blank();
 		this.dependsOn = blank();
+		this.stronglyDependsOn = blank();
 
 		this.isIncluded = false;
 
@@ -136,10 +136,15 @@ export default class Statement {
 				return;
 			}
 
+			// disregard the `bar` in `class Foo { bar () {...} }`
+			if ( parent.type === 'MethodDefinition' ) return;
+
 			const definingScope = scope.findDefiningScope( node.name );
 
 			if ( ( !definingScope || definingScope.depth === 0 ) && !this.defines[ node.name ] ) {
 				this.dependsOn[ node.name ] = true;
+
+				if ( !scope.parent ) this.stronglyDependsOn[ node.name ] = true;
 			}
 		}
 	}
@@ -206,8 +211,7 @@ export default class Statement {
 	}
 
 	expand () {
-		if ( this.isIncluded ) return emptyArrayPromise; // TODO can this happen?
-		this.isIncluded = true;
+		this.isIncluded = true; // prevent statement being included twice
 
 		let result = [];
 
@@ -216,6 +220,8 @@ export default class Statement {
 		const dependencies = Object.keys( this.dependsOn );
 
 		return sequence( dependencies, name => {
+			if ( this.defines[ name ] ) return; // TODO maybe exclude from `this.dependsOn` in the first place?
+
 			return this.module.define( name ).then( definition => {
 				result.push.apply( result, definition );
 			});

test/function/cycles-pathological/A.js

@@ -0,0 +1,11 @@
+import B from './B';
+
+export default class A {
+	constructor () {
+		this.isA = true;
+	}
+
+	b () {
+		return new B();
+	}
+}

test/function/cycles-pathological/B.js

@@ -0,0 +1,8 @@
+import A from './A';
+
+export default class B extends A {
+	constructor () {
+		super();
+		this.isB = true;
+	}
+}

test/function/cycles-pathological/C.js

@@ -0,0 +1,8 @@
+import D from './D';
+
+export default class C extends D {
+	constructor () {
+		super();
+		this.isC = true;
+	}
+}

test/function/cycles-pathological/D.js

@@ -0,0 +1,11 @@
+import C from './C';
+
+export default class D {
+	constructor () {
+		this.isD = true;
+	}
+
+	c () {
+		return new C();
+	}
+}

test/function/cycles-pathological/_config.js

@@ -0,0 +1,18 @@
+var assert = require( 'assert' );
+
+module.exports = {
+	description: 'resolves pathological cyclical dependencies gracefully',
+	babel: true,
+	exports: function ( exports ) {
+		assert.ok( exports.a.isA );
+		assert.ok( exports.b1.isA );
+		assert.ok( exports.b1.isB );
+		assert.ok( exports.b2.isA );
+		assert.ok( exports.b2.isB );
+		assert.ok( exports.c1.isC );
+		assert.ok( exports.c1.isD );
+		assert.ok( exports.c2.isC );
+		assert.ok( exports.c2.isD );
+		assert.ok( exports.d.isD );
+	}
+};

test/function/cycles-pathological/main.js

@@ -0,0 +1,12 @@
+import A from './A';
+import B from './B';
+
+import C from './C';
+import D from './D';
+
+export const a = new A();
+export const b1 = a.b();
+export const b2 = new B();
+export const c1 = new C();
+export const d = new D();
+export const c2 = d.c();