Merge branch 'master' into gh-34

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: '' });
 

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();

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 );
 			});

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 );
 }

src/utils/resolvePath.js → 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

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;
 			}

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 );
+	}
+};

test/function/custom-path-resolver-on-entry/bar.js

@@ -0,0 +1,3 @@
+export default function () {
+	return 21;
+}

test/function/custom-path-resolver-on-entry/foo.js

@@ -0,0 +1,5 @@
+import bar from './bar';
+
+export default function () {
+	return bar() * 2;
+}

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;
 		}

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();
+	}
+}

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;
+	}
+}

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;
+	}
+}

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();
+	}
+}

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 );
+	}
+};

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();