From ce3d61745b05ef3a6e1c6ce4a565118da4dcc0b6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 18 May 2015 16:03:16 -0400 Subject: [PATCH] better handling of external modules --- src/Bundle.js | 38 +++++++++---------- src/Module.js | 3 +- src/utils/resolvePath.js | 11 ++++++ .../_config.js | 3 ++ .../foo.js | 6 +++ .../main.js | 8 ++++ .../allows-external-modules/_config.js | 2 +- .../export-from-internal-module/_config.js | 3 +- 8 files changed, 49 insertions(+), 25 deletions(-) create mode 100644 src/utils/resolvePath.js create mode 100644 test/samples/allows-external-modules-from-nested-module/_config.js create mode 100644 test/samples/allows-external-modules-from-nested-module/foo.js create mode 100644 test/samples/allows-external-modules-from-nested-module/main.js diff --git a/src/Bundle.js b/src/Bundle.js index e106c29..e5794d3 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -8,13 +8,16 @@ import ExternalModule from './ExternalModule'; import finalisers from './finalisers/index'; import replaceIdentifiers from './utils/replaceIdentifiers'; import makeLegalIdentifier from './utils/makeLegalIdentifier'; +import { defaultResolver } from './utils/resolvePath'; export default class Bundle { constructor ( options ) { - this.base = options.base; - this.entryPath = resolve( this.base, options.entry ); + this.base = options.base || process.cwd(); + this.entryPath = resolve( this.base, options.entry ).replace( /\.js$/, '' ) + '.js'; this.entryModule = null; + this.resolvePath = options.resolvePath || defaultResolver; + this.modulePromises = {}; this.modules = {}; @@ -28,9 +31,19 @@ export default class Bundle { this.externalModules = []; } - fetchModule ( path, id ) { - // TODO currently, we'll get different ExternalModule objects - // depending on where they're imported from... + fetchModule ( importee, importer ) { + const path = this.resolvePath( importee, importer ); + + if ( !path ) { + // external module + if ( !has( this.modulePromises, importee ) ) { + const module = new ExternalModule( importee ); + this.externalModules.push( module ); + this.modulePromises[ importee ] = Promise.resolve( module ); + } + + return this.modulePromises[ importee ]; + } if ( !has( this.modulePromises, path ) ) { this.modulePromises[ path ] = readFile( path, { encoding: 'utf-8' }) @@ -43,21 +56,6 @@ export default class Bundle { this.modules[ path ] = module; return module; - }, err => { - if ( err.code === 'ENOENT' ) { - if ( id[0] === '.' ) { - // external modules can't have relative paths - throw err; - } - - // most likely an external module - // TODO fire an event, or otherwise allow some way for - // users to control external modules better? - const module = new ExternalModule( id ); - - this.externalModules.push( module ); - return module; - } }); } diff --git a/src/Module.js b/src/Module.js index 3c0ecdd..bbe3dd7 100644 --- a/src/Module.js +++ b/src/Module.js @@ -184,9 +184,8 @@ export default class Module { // The definition for this name is in a different module if ( has( this.imports, name ) ) { const importDeclaration = this.imports[ name ]; - const path = resolve( dirname( this.path ), importDeclaration.source ) + '.js'; - promise = this.bundle.fetchModule( path, importDeclaration.source ) + promise = this.bundle.fetchModule( importDeclaration.source, this.path ) .then( module => { importDeclaration.module = module; diff --git a/src/utils/resolvePath.js b/src/utils/resolvePath.js new file mode 100644 index 0000000..4b07512 --- /dev/null +++ b/src/utils/resolvePath.js @@ -0,0 +1,11 @@ +import { dirname, isAbsolute, resolve } from 'path'; + +export function defaultResolver ( importee, importer ) { + // absolute paths are left untouched + if ( isAbsolute( importee ) ) return importee; + + // external modules stay external + if ( importee[0] !== '.' ) return false; + + return resolve( dirname( importer ), importee ).replace( /\.js$/, '' ) + '.js'; +} \ No newline at end of file diff --git a/test/samples/allows-external-modules-from-nested-module/_config.js b/test/samples/allows-external-modules-from-nested-module/_config.js new file mode 100644 index 0000000..3432ce7 --- /dev/null +++ b/test/samples/allows-external-modules-from-nested-module/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'imports external modules from nested internal modules' +}; \ No newline at end of file diff --git a/test/samples/allows-external-modules-from-nested-module/foo.js b/test/samples/allows-external-modules-from-nested-module/foo.js new file mode 100644 index 0000000..f1b013a --- /dev/null +++ b/test/samples/allows-external-modules-from-nested-module/foo.js @@ -0,0 +1,6 @@ +import { relative } from 'path'; + +var path = 'a/b/c'; +var path2 = 'a/c/b'; + +export default relative( path, path2 ); \ No newline at end of file diff --git a/test/samples/allows-external-modules-from-nested-module/main.js b/test/samples/allows-external-modules-from-nested-module/main.js new file mode 100644 index 0000000..8bacdc9 --- /dev/null +++ b/test/samples/allows-external-modules-from-nested-module/main.js @@ -0,0 +1,8 @@ +import { relative } from 'path'; +import foo from './foo'; + +var path = 'foo/bar/baz'; +var path2 = 'foo/baz/bar'; + +assert.equal( relative( path, path2 ), '../../baz/bar' ); +assert.equal( foo, '../../c/b' ); \ No newline at end of file diff --git a/test/samples/allows-external-modules/_config.js b/test/samples/allows-external-modules/_config.js index 3366573..b2aae64 100644 --- a/test/samples/allows-external-modules/_config.js +++ b/test/samples/allows-external-modules/_config.js @@ -1,3 +1,3 @@ module.exports = { - description: 'Non-existent modules are assumed to be external' + description: 'Non-absolute and non-relative modules are assumed to be external' }; \ No newline at end of file diff --git a/test/samples/export-from-internal-module/_config.js b/test/samples/export-from-internal-module/_config.js index cc774bc..b09fc42 100644 --- a/test/samples/export-from-internal-module/_config.js +++ b/test/samples/export-from-internal-module/_config.js @@ -2,6 +2,5 @@ module.exports = { description: 'exports from an internal module', exports: function ( exports, assert ) { assert.equal( exports.foo, 42 ); - }, - solo: true + } }; \ No newline at end of file