find importer of badly-ordered modules in order to provide useful feedback

src/Bundle.js

@@ -169,7 +169,7 @@ export default class Bundle {
 	}
 
 	fetchAllDependencies ( module ) {
-		const promises = module.dependencies.map( source => {
+		const promises = module.sources.map( source => {
 			return this.resolveId( source, module.id )
 				.then( resolvedId => {
 					// If the `resolvedId` is supposed to be external, make it so.
@@ -276,60 +276,83 @@ export default class Bundle {
 	}
 
 	sort () {
-		const moduleById = this.moduleById;
-
 		let seen = {};
+		let hasCycles;
 		let ordered = [];
 
 		let stronglyDependsOn = blank();
+		let dependsOn = blank();
 
 		this.modules.forEach( module => {
 			stronglyDependsOn[ module.id ] = blank();
+			dependsOn[ module.id ] = blank();
 		});
 
 		this.modules.forEach( module => {
-			function processDependency ( dependency ) {
+			function processStrongDependency ( dependency ) {
 				if ( dependency === module || stronglyDependsOn[ module.id ][ dependency.id ] ) return;
 
 				stronglyDependsOn[ module.id ][ dependency.id ] = true;
-				dependency.strongDependencies.forEach( processDependency );
+				dependency.strongDependencies.forEach( processStrongDependency );
+			}
+
+			function processDependency ( dependency ) {
+				if ( dependency === module || dependsOn[ module.id ][ dependency.id ] ) return;
+
+				dependsOn[ module.id ][ dependency.id ] = true;
+				dependency.dependencies.forEach( processDependency );
 			}
 
-			module.strongDependencies.forEach( processDependency );
+			module.strongDependencies.forEach( processStrongDependency );
+			module.dependencies.forEach( processDependency );
 		});
 
 		const visit = module => {
-			if ( seen[ module.id ] ) return;
-			seen[ module.id ] = true;
+			if ( seen[ module.id ] ) {
+				hasCycles = true;
+				return;
+			}
 
-			const dependencies = module.dependencies
-				.map( source => {
-					const resolved = module.resolvedIds[ source ];
-					return moduleById[ resolved ];
-				})
-				.filter( dependency => !dependency.isExternal );
+			seen[ module.id ] = true;
 
-			dependencies.forEach( visit );
+			module.dependencies.forEach( visit );
 			ordered.push( module );
 		};
 
 		visit( this.entryModule );
 
-		ordered.forEach( ( a, i ) => {
-			const b = ordered[ i + 1 ];
-			if ( !b ) return;
-
-			if ( stronglyDependsOn[ a.id ][ b.id ] ) {
-				// somewhere, there is a module that imports b before a. Because
-				// b imports a, a is placed before b. We need to find the module
-				// in question, so we can provide a useful error message
-				const parent = { id: 'TODO' };
+		if ( hasCycles ) {
+			ordered.forEach( ( a, i ) => {
+				for ( i += 1; i < ordered.length; i += 1 ) {
+					const b = ordered[i];
+
+					if ( stronglyDependsOn[ a.id ][ b.id ] ) {
+						// somewhere, there is a module that imports b before a. Because
+						// b imports a, a is placed before b. We need to find the module
+						// in question, so we can provide a useful error message
+						let parent = '[[unknown]]';
+
+						const findParent = module => {
+							if ( dependsOn[ module.id ][ a.id ] && dependsOn[ module.id ][ b.id ] ) {
+								parent = module.id;
+							} else {
+								for ( let i = 0; i < module.dependencies.length; i += 1 ) {
+									const dependency = module.dependencies[i];
+									if ( findParent( dependency ) ) return;
+								}
+							}
+						};
+
+						findParent( this.entryModule );
+
+						throw new Error(
+							`Module ${a.id} may be unable to evaluate without ${b.id}, but is included first due to a cyclical dependency. Consider swapping the import statements in ${parent} to ensure correct ordering`
+						);
+					}
+				}
+			});
+		}
 
-				throw new Error(
-					`Module ${a.id} may be unable to evaluate without ${b.id}, but is included first due to a cyclical dependency. Consider swapping the import statements in ${parent.id} to ensure correct ordering`
-				);
-			}
-		});
 
 		return ordered;
 	}

src/Module.js

@@ -22,6 +22,7 @@ export default class Module {
 		this.id = id;
 
 		// all dependencies
+		this.sources = [];
 		this.dependencies = [];
 		this.resolvedIds = blank();
 
@@ -62,7 +63,7 @@ export default class Module {
 
 		// export { name } from './other.js'
 		if ( source ) {
-			if ( !~this.dependencies.indexOf( source ) ) this.dependencies.push( source );
+			if ( !~this.sources.indexOf( source ) ) this.sources.push( source );
 
 			if ( node.type === 'ExportAllDeclaration' ) {
 				// Store `export * from '...'` statements in an array of delegates.
@@ -136,7 +137,7 @@ export default class Module {
 		const node = statement.node;
 		const source = node.source.value;
 
-		if ( !~this.dependencies.indexOf( source ) ) this.dependencies.push( source );
+		if ( !~this.sources.indexOf( source ) ) this.sources.push( source );
 
 		node.specifiers.forEach( specifier => {
 			const localName = specifier.local.name;
@@ -212,6 +213,13 @@ export default class Module {
 			const id = this.resolvedIds[ source ];
 			return this.bundle.moduleById[ id ];
 		});
+
+		this.sources.forEach( source => {
+			const id = this.resolvedIds[ source ];
+			const module = this.bundle.moduleById[ id ];
+
+			if ( !module.isExternal ) this.dependencies.push( module );
+		});
 	}
 
 	bindReferences () {
@@ -243,26 +251,6 @@ export default class Module {
 		});
 	}
 
-	consolidateDependencies () {
-		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 && !~weakDependencies.indexOf( dependency ) ) {
-				weakDependencies.push( dependency );
-			}
-		});
-
-		strongDependencies = this.strongDependencies
-			.filter( module => module !== this );
-
-		return { strongDependencies, weakDependencies };
-	}
-
 	getExports () {
 		let exports = blank();