From 995737eb6d022c3e2278bb4c9d485e7723c95785 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sun, 10 Jan 2016 12:36:27 -0500 Subject: [PATCH] find importer of badly-ordered modules in order to provide useful feedback --- src/Bundle.js | 81 +++++++++++++++++++++++++++++++++------------------ src/Module.js | 32 +++++++------------- 2 files changed, 62 insertions(+), 51 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index b38ff21..a9885c4 100644 --- a/src/Bundle.js +++ b/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; } diff --git a/src/Module.js b/src/Module.js index 38f63e0..324b4b8 100644 --- a/src/Module.js +++ b/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();