diff --git a/src/Bundle.js b/src/Bundle.js index 9d22e87..8e18fcb 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -1,4 +1,4 @@ -import { basename, dirname, extname, relative, resolve } from 'path'; +import { basename, dirname, extname, relative } from 'path'; import { Promise } from 'sander'; import MagicString from 'magic-string'; import { blank, keys } from './utils/object'; @@ -7,7 +7,7 @@ import ExternalModule from './ExternalModule'; import finalisers from './finalisers/index'; import makeLegalIdentifier from './utils/makeLegalIdentifier'; import ensureArray from './utils/ensureArray'; -import { defaultResolver, defaultExternalResolver } from './utils/resolvePath'; +import { defaultResolver, defaultExternalResolver } from './utils/resolveId'; import { defaultLoader } from './utils/load'; import getExportMode from './utils/getExportMode'; import getIndentString from './utils/getIndentString'; @@ -15,13 +15,13 @@ import { unixizePath } from './utils/normalizePlatform.js'; export default class Bundle { constructor ( options ) { - this.entryPath = resolve( options.entry ).replace( /\.js$/, '' ) + '.js'; - this.base = dirname( this.entryPath ); + this.entry = options.entry; + this.entryModule = null; - this.resolvePath = options.resolvePath || defaultResolver; + this.resolveId = options.resolveId || defaultResolver; this.load = options.load || defaultLoader; - this.resolvePathOptions = { + this.resolveOptions = { external: ensureArray( options.external ), resolveExternal: options.resolveExternal || defaultExternalResolver }; @@ -30,8 +30,6 @@ export default class Bundle { transform: ensureArray( options.transform ) }; - this.entryModule = null; - this.varExports = blank(); this.toExport = null; @@ -43,9 +41,9 @@ export default class Bundle { } fetchModule ( importee, importer ) { - return Promise.resolve( importer === null ? importee : this.resolvePath( importee, importer, this.resolvePathOptions ) ) - .then( path => { - if ( !path ) { + return Promise.resolve( this.resolveId( importee, importer, this.resolveOptions ) ) + .then( id => { + if ( !id ) { // external module if ( !this.modulePromises[ importee ] ) { const module = new ExternalModule( importee ); @@ -56,11 +54,11 @@ export default class Bundle { return this.modulePromises[ importee ]; } - if ( !this.modulePromises[ path ] ) { - this.modulePromises[ path ] = Promise.resolve( this.load( path, this.loadOptions ) ) + if ( !this.modulePromises[ id ] ) { + this.modulePromises[ id ] = Promise.resolve( this.load( id, this.loadOptions ) ) .then( source => { const module = new Module({ - path, + id, source, bundle: this }); @@ -69,13 +67,13 @@ export default class Bundle { }); } - return this.modulePromises[ path ]; + return this.modulePromises[ id ]; }); } build () { // bring in top-level AST nodes from the entry module - return this.fetchModule( this.entryPath, null ) + return this.fetchModule( this.entry, undefined ) .then( entryModule => { const defaultExport = entryModule.exports.default; @@ -89,9 +87,9 @@ export default class Bundle { } // `export default a + b` - generate an export name - // based on the filename of the entry module + // based on the id of the entry module else { - let defaultExportName = makeLegalIdentifier( basename( this.entryPath ).slice( 0, -extname( this.entryPath ).length ) ); + let defaultExportName = makeLegalIdentifier( basename( this.entryModule.id ).slice( 0, -extname( this.entryModule.id ).length ) ); // deconflict let topLevelNames = []; @@ -112,6 +110,7 @@ export default class Bundle { .then( statements => { this.statements = statements; this.deconflict(); + this.sort(); }); } @@ -200,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: '' }); diff --git a/src/Module.js b/src/Module.js index 83e87bc..4297fa4 100644 --- a/src/Module.js +++ b/src/Module.js @@ -1,3 +1,4 @@ +import { dirname } from 'path'; import { Promise } from 'sander'; import { parse } from 'acorn'; import MagicString from 'magic-string'; @@ -21,14 +22,16 @@ function deconflict ( name, names ) { } export default class Module { - constructor ({ path, source, bundle }) { + constructor ({ id, source, bundle }) { this.source = source; this.bundle = bundle; - this.path = path; + this.id = id; + // By default, `id` is the filename. Custom resolvers and loaders + // can change that, but it makes sense to use it for the source filename this.magicString = new MagicString( source, { - filename: path + filename: id }); this.suggestedNames = blank(); @@ -46,7 +49,7 @@ export default class Module { }); } catch ( err ) { err.code = 'PARSE_ERROR'; - err.file = path; + err.file = id; // see above - not necessarily true, but true enough throw err; } @@ -113,7 +116,7 @@ export default class Module { if ( this.imports[ localName ] ) { const err = new Error( `Duplicated import '${localName}'` ); - err.file = this.path; + err.file = this.id; err.loc = getLocation( this.source, specifier.start ); throw err; } @@ -257,8 +260,9 @@ export default class Module { } getCanonicalName ( localName ) { + // Special case if ( localName === 'default' && ( this.exports.default.isModified || !this.suggestedNames.default ) ) { - let canonicalName = makeLegalIdentifier( this.path.replace( this.bundle.base + '/', '' ).replace( /\.js$/, '' ) ); + let canonicalName = makeLegalIdentifier( this.id.replace( dirname( this.bundle.entryModule.id ) + '/', '' ).replace( /\.js$/, '' ) ); return deconflict( canonicalName, this.definitions ); } @@ -311,7 +315,7 @@ export default class Module { if ( this.imports[ name ] ) { const importDeclaration = this.imports[ name ]; - promise = this.bundle.fetchModule( importDeclaration.source, this.path ) + promise = this.bundle.fetchModule( importDeclaration.source, this.id ) .then( module => { importDeclaration.module = module; @@ -357,7 +361,7 @@ export default class Module { const exportDeclaration = module.exports[ importDeclaration.name ]; if ( !exportDeclaration ) { - throw new Error( `Module ${module.path} does not export ${importDeclaration.name} (imported by ${this.path})` ); + throw new Error( `Module ${module.id} does not export ${importDeclaration.name} (imported by ${this.id})` ); } return module.define( exportDeclaration.localName ); @@ -432,7 +436,7 @@ export default class Module { // ...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 ) { - return this.bundle.fetchModule( statement.node.source.value, this.path ) + return this.bundle.fetchModule( statement.node.source.value, this.id ) .then( module => { statement.module = module; return module.expandAllStatements(); diff --git a/src/Statement.js b/src/Statement.js index 9a1933f..3d67174 100644 --- a/src/Statement.js +++ b/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; } } } @@ -164,7 +169,7 @@ export default class Statement { if ( depth < minDepth ) { const err = new Error( `Illegal reassignment to import '${node.name}'` ); - err.file = this.module.path; + err.file = this.module.id; err.loc = getLocation( this.module.magicString.toString(), node.start ); throw err; } @@ -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 ); }); diff --git a/src/utils/load.js b/src/utils/load.js index 9b79d9a..60b7566 100644 --- a/src/utils/load.js +++ b/src/utils/load.js @@ -1,10 +1,10 @@ import { readFileSync } from 'sander'; -export function defaultLoader ( path, options ) { +export function defaultLoader ( id, options ) { // TODO support plugins e.g. !css and !json? - const source = readFileSync( path, { encoding: 'utf-8' }); + const source = readFileSync( id, { encoding: 'utf-8' }); return options.transform.reduce( ( source, transformer ) => { - return transformer( source, path ); + return transformer( source, id ); }, source ); } diff --git a/src/utils/resolvePath.js b/src/utils/resolveId.js similarity index 93% rename from src/utils/resolvePath.js rename to src/utils/resolveId.js index 77fe421..5f9de68 100644 --- a/src/utils/resolvePath.js +++ b/src/utils/resolveId.js @@ -7,6 +7,9 @@ export function defaultResolver ( importee, importer, options ) { // absolute paths are left untouched if ( absolutePath.test( importee ) ) return importee; + // if this is the entry point, resolve against cwd + if ( importer === undefined ) return resolve( importee ); + // we try to resolve external modules if ( importee[0] !== '.' ) { // unless we want to keep it external, that is diff --git a/test/function/custom-path-resolver-async/_config.js b/test/function/custom-path-resolver-async/_config.js index acb62d0..287f536 100644 --- a/test/function/custom-path-resolver-async/_config.js +++ b/test/function/custom-path-resolver-async/_config.js @@ -1,14 +1,17 @@ +var path = require( 'path' ); var assert = require( 'assert' ); module.exports = { description: 'uses a custom path resolver (asynchronous)', options: { - resolvePath: function ( importee, importer ) { + resolveId: function ( importee, importer ) { var Promise = require( 'sander' ).Promise; var resolved; + if ( importee === path.resolve( __dirname, 'main.js' ) ) return importee; + if ( importee === 'foo' ) { - resolved = require( 'path' ).resolve( __dirname, 'bar.js' ); + resolved = path.resolve( __dirname, 'bar.js' ); } else { resolved = false; } diff --git a/test/function/custom-path-resolver-on-entry/_config.js b/test/function/custom-path-resolver-on-entry/_config.js new file mode 100644 index 0000000..83a6a67 --- /dev/null +++ b/test/function/custom-path-resolver-on-entry/_config.js @@ -0,0 +1,35 @@ +var path = require( 'path' ); +var fs = require( 'fs' ); +var assert = require( 'assert' ); + +var cachedModules = { + '@main.js': 'import foo from "./foo"; export default foo();' +}; + +module.exports = { + description: 'applies custom resolver to entry point', + //solo: true, + options: { + resolveId: function ( importee, importer ) { + if ( importer === undefined ) { + return '@' + path.relative( __dirname, importee ); + } + + if ( importer[0] === '@' ) { + return path.resolve( __dirname, importee ) + '.js'; + } + + return path.resolve( path.dirname( importer ), importee ) + '.js'; + }, + load: function ( moduleId ) { + if ( moduleId[0] === '@' ) { + return cachedModules[ moduleId ]; + } + + return fs.readFileSync( moduleId, 'utf-8' ); + } + }, + exports: function ( exports ) { + assert.equal( exports, 42 ); + } +}; diff --git a/test/function/custom-path-resolver-on-entry/bar.js b/test/function/custom-path-resolver-on-entry/bar.js new file mode 100644 index 0000000..562d38d --- /dev/null +++ b/test/function/custom-path-resolver-on-entry/bar.js @@ -0,0 +1,3 @@ +export default function () { + return 21; +} diff --git a/test/function/custom-path-resolver-on-entry/foo.js b/test/function/custom-path-resolver-on-entry/foo.js new file mode 100644 index 0000000..642dadd --- /dev/null +++ b/test/function/custom-path-resolver-on-entry/foo.js @@ -0,0 +1,5 @@ +import bar from './bar'; + +export default function () { + return bar() * 2; +} diff --git a/test/function/custom-path-resolver-sync/_config.js b/test/function/custom-path-resolver-sync/_config.js index a7cd96b..9a755e5 100644 --- a/test/function/custom-path-resolver-sync/_config.js +++ b/test/function/custom-path-resolver-sync/_config.js @@ -1,12 +1,12 @@ +var path = require( 'path' ); var assert = require( 'assert' ); module.exports = { description: 'uses a custom path resolver (synchronous)', options: { - resolvePath: function ( importee, importer ) { - if ( importee === 'foo' ) { - return require( 'path' ).resolve( __dirname, 'bar.js' ); - } + resolveId: function ( importee, importer ) { + if ( importee === path.resolve( __dirname, 'main.js' ) ) return importee; + if ( importee === 'foo' ) return path.resolve( __dirname, 'bar.js' ); return false; } diff --git a/test/function/cycles-pathological/A.js b/test/function/cycles-pathological/A.js new file mode 100644 index 0000000..9e0fd2e --- /dev/null +++ b/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(); + } +} diff --git a/test/function/cycles-pathological/B.js b/test/function/cycles-pathological/B.js new file mode 100644 index 0000000..1c8e9b4 --- /dev/null +++ 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; + } +} diff --git a/test/function/cycles-pathological/C.js b/test/function/cycles-pathological/C.js new file mode 100644 index 0000000..313bba7 --- /dev/null +++ b/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; + } +} diff --git a/test/function/cycles-pathological/D.js b/test/function/cycles-pathological/D.js new file mode 100644 index 0000000..2d8a405 --- /dev/null +++ b/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(); + } +} diff --git a/test/function/cycles-pathological/_config.js b/test/function/cycles-pathological/_config.js new file mode 100644 index 0000000..156a2f3 --- /dev/null +++ b/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 ); + } +}; diff --git a/test/function/cycles-pathological/main.js b/test/function/cycles-pathological/main.js new file mode 100644 index 0000000..f4c19db --- /dev/null +++ b/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();