From 477324e7531978e6556a394bbe122a4daab12b25 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Sat, 22 Aug 2015 18:00:47 -0400 Subject: [PATCH] more efficient dependency fetching --- src/Bundle.js | 53 +++++++++++++++++++++++---------------------------- src/Module.js | 50 +++++++++++++++++++++++++++++++----------------- 2 files changed, 56 insertions(+), 47 deletions(-) diff --git a/src/Bundle.js b/src/Bundle.js index 6111f08..2f816b8 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -197,41 +197,36 @@ export default class Bundle { this.modules.push( module ); this.moduleById[ id ] = module; - const promises = this.fetchAllModules( module, module.imports ) - .concat( this.fetchAllModules( module, module.reexports ) ); - - return Promise.all( promises ).then( () => module ); + return this.fetchAllDependencies( module ).then( () => module ); }); } - fetchAllModules ( module, specifiers ) { - return keys( specifiers ) - .map( name => { - const specifier = specifiers[ name ]; - - return Promise.resolve( this.resolveId( specifier.source, module.id, this.resolveOptions ) ) - .then( resolvedId => { - // external module - if ( !resolvedId ) { - if ( !this.moduleById[ specifier.source ] ) { - const module = new ExternalModule( specifier.source ); - this.externalModules.push( module ); - this.moduleById[ specifier.source ] = module; - } - - specifier.module = this.moduleById[ specifier.source ]; + fetchAllDependencies ( module ) { + const promises = module.dependencies.map( source => { + return Promise.resolve( this.resolveId( source, module.id, this.resolveOptions ) ) + .then( resolvedId => { + module.resolvedIds[ source ] = resolvedId || source; + + // external module + if ( !resolvedId ) { + if ( !this.moduleById[ source ] ) { + const module = new ExternalModule( source ); + this.externalModules.push( module ); + this.moduleById[ source ] = module; } + } - else if ( resolvedId === module.id ) { - throw new Error( `A module cannot import itself (${resolvedId})` ); - } + else if ( resolvedId === module.id ) { + throw new Error( `A module cannot import itself (${resolvedId})` ); + } - else { - specifier.id = resolvedId; - return this.fetchModule( resolvedId ); - } - }); - }); + else { + return this.fetchModule( resolvedId ); + } + }); + }); + + return Promise.all( promises ); } markAllModifierStatements () { diff --git a/src/Module.js b/src/Module.js index b48995b..e07e363 100644 --- a/src/Module.js +++ b/src/Module.js @@ -52,7 +52,12 @@ export default class Module { this.statements = this.parse( ast ); - // imports and exports, indexed by ID + // all dependencies + this.dependencies = []; + this.resolvedIds = blank(); + this.boundImportSpecifiers = false; + + // imports and exports, indexed by local name this.imports = blank(); this.exports = blank(); this.reexports = blank(); @@ -80,6 +85,8 @@ export default class Module { // export { name } from './other' if ( source ) { + if ( !~this.dependencies.indexOf( source ) ) this.dependencies.push( source ); + if ( node.type === 'ExportAllDeclaration' ) { // Store `export * from '...'` statements in an array of delegates. // When an unknown import is encountered, we see if one of them can satisfy it. @@ -168,6 +175,8 @@ export default class Module { const node = statement.node; const source = node.source.value; + if ( !~this.dependencies.indexOf( source ) ) this.dependencies.push( source ); + node.specifiers.forEach( specifier => { const isDefault = specifier.type === 'ImportDefaultSpecifier'; const isNamespace = specifier.type === 'ImportNamespaceSpecifier'; @@ -226,16 +235,26 @@ export default class Module { } bindImportSpecifiers () { + if ( this.boundImportSpecifiers ) return; + this.boundImportSpecifiers = true; + [ this.imports, this.reexports ].forEach( specifiers => { keys( specifiers ).forEach( name => { const specifier = specifiers[ name ]; if ( specifier.module ) return; - specifier.module = this.bundle.moduleById[ specifier.id ]; - specifier.module.bindImportSpecifiers(); + const id = this.resolvedIds[ specifier.source ]; + specifier.module = this.bundle.moduleById[ id ]; }); }); + + this.dependencies.forEach( source => { + const id = this.resolvedIds[ source ]; + const module = this.bundle.moduleById[ id ]; + + if ( !module.isExternal ) module.bindImportSpecifiers(); + }); } consolidateDependencies () { @@ -249,9 +268,12 @@ export default class Module { } this.statements.forEach( statement => { - if ( statement.isImportDeclaration && !statement.node.specifiers.length && !statement.module.isExternal ) { + 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? + const id = this.resolvedIds[ statement.node.source.value ]; + const module = this.bundle.moduleById[ id ]; + + if ( !module.isExternal ) strongDependencies[ module.id ] = module; } else if ( statement.isReexportDeclaration ) { @@ -390,20 +412,12 @@ export default class Module { if ( statement.isImportDeclaration ) { // ...unless they're empty, in which case assume we're importing them for the side-effects // THIS IS NOT FOOLPROOF. Probably need /*rollup: include */ or similar + if ( !statement.node.specifiers.length ) { + const id = this.resolvedIds[ statement.node.source.value ]; + const otherModule = this.bundle.moduleById[ id ]; - // TODO... - // if ( !statement.node.specifiers.length ) { - // return this.bundle.fetchModule( statement.node.source.value, this.id ) - // .then( module => { - // statement.module = module; - // if ( module.isExternal ) { - // return; - // } - // return module.markAllStatements(); - // }); - // } - // - // return; + if ( !otherModule.isExternal ) otherModule.markAllStatements(); + } } // skip `export { foo, bar, baz }`...