Incremental external (#2718)

* Added failing test for incremental build with externals

* Let resolvedIds track both id and whether external

src/Graph.ts

@@ -571,7 +571,7 @@ export default class Graph {
 						}
 					}
 					module.exportAllSources.forEach(source => {
-						const id = module.resolvedIds[source];
+						const id = module.resolvedIds[source].id;
 						const exportAllModule = this.moduleById.get(id);
 						if (exportAllModule.isExternal) return;
 
@@ -642,8 +642,8 @@ export default class Graph {
 			module.sources.map(source => {
 				return Promise.resolve()
 					.then(() => {
-						const resolvedId = module.resolvedIds[source];
-						if (resolvedId) return resolvedId;
+						const resolved = module.resolvedIds[source];
+						if (resolved) return !resolved.external && resolved.id;
 						if (this.isExternal(source, module.id, false)) return false;
 						return this.pluginDriver.hookFirst<string | boolean | void>('resolveId', [
 							source,
@@ -682,7 +682,7 @@ export default class Graph {
 						}
 
 						if (isExternal) {
-							module.resolvedIds[source] = externalId;
+							module.resolvedIds[source] = { id: externalId, external: true };
 
 							if (!this.moduleById.has(externalId)) {
 								const module = new ExternalModule({ graph: this, id: externalId });
@@ -709,7 +709,7 @@ export default class Graph {
 								externalModule.getVariableForExportName(importDeclaration.name);
 							}
 						} else {
-							module.resolvedIds[source] = <string>resolvedId;
+							module.resolvedIds[source] = { id: <string>resolvedId, external: false };
 							return this.fetchModule(<string>resolvedId, module.id);
 						}
 					});

src/Module.ts

@@ -471,7 +471,7 @@ export default class Module {
 
 	linkDependencies() {
 		for (const source of this.sources) {
-			const id = this.resolvedIds[source];
+			const id = this.resolvedIds[source].id;
 
 			if (id) {
 				const module = this.graph.moduleById.get(id);
@@ -488,7 +488,7 @@ export default class Module {
 		this.addModulesToSpecifiers(this.reexports);
 
 		this.exportAllModules = this.exportAllSources.map(source => {
-			const id = this.resolvedIds[source];
+			const id = this.resolvedIds[source].id;
 			return this.graph.moduleById.get(id);
 		});
 	}
@@ -498,7 +498,8 @@ export default class Module {
 	}) {
 		for (const name of Object.keys(specifiers)) {
 			const specifier = specifiers[name];
-			specifier.module = this.graph.moduleById.get(this.resolvedIds[specifier.source]);
+			const id = this.resolvedIds[specifier.source].id;
+			specifier.module = this.graph.moduleById.get(id);
 		}
 	}
 

src/rollup/types.d.ts

@@ -4,7 +4,7 @@ import { EventEmitter } from 'events';
 export const VERSION: string;
 
 export interface IdMap {
-	[key: string]: string;
+	[key: string]: { id: string; external: boolean };
 }
 
 export interface RollupError extends RollupLogProps {

src/utils/pluginDriver.ts

@@ -148,7 +148,7 @@ export function createPluginDriver(
 					isExternal: !!foundModule.isExternal,
 					importedIds: foundModule.isExternal
 						? []
-						: (foundModule as Module).sources.map(id => (foundModule as Module).resolvedIds[id])
+						: (foundModule as Module).sources.map(id => (foundModule as Module).resolvedIds[id].id)
 				};
 			},
 			watcher: watcher

test/incremental/index.js

@@ -11,7 +11,7 @@ describe('incremental', () => {
 	const plugin = {
 		resolveId: id => {
 			resolveIdCalls += 1;
-			return id;
+			return id === 'external' ? false : id;
 		},
 
 		load: id => {
@@ -131,6 +131,42 @@ describe('incremental', () => {
 			});
 	});
 
+	it('respects externals from resolveId', () => {
+		let cache;
+		modules.foo = `import p from 'external'; export default p;`;
+
+		const require = id => id === 'external' && 43;
+
+		return rollup
+			.rollup({
+				input: 'entry',
+				plugins: [plugin]
+			})
+			.then(bundle => {
+				assert.equal(resolveIdCalls, 3);
+
+				return executeBundle(bundle, require).then(result => {
+					assert.equal(result, 43);
+					cache = bundle.cache;
+				});
+			})
+			.then(() => {
+				return rollup.rollup({
+					input: 'entry',
+					plugins: [plugin],
+					cache
+				});
+			})
+			.then(bundle => {
+				assert.equal(resolveIdCalls, 4);
+
+				return executeBundle(bundle, require);
+			})
+			.then(result => {
+				assert.equal(result, 43);
+			});
+	});
+
 	it('keeps ASTs between runs', () => {
 		return rollup
 			.rollup({
@@ -204,8 +240,8 @@ describe('incremental', () => {
 				assert.equal(bundle.cache.modules[0].id, 'entry');
 
 				assert.deepEqual(bundle.cache.modules[0].resolvedIds, {
-					foo: 'foo',
-					external: 'external'
+					foo: { id: 'foo', external: false },
+					external: { id: 'external', external: true }
 				});
 			});
 	});

test/utils.js

@@ -59,16 +59,16 @@ function deindent(str) {
 		.trim();
 }
 
-function executeBundle(bundle) {
+function executeBundle(bundle, require) {
 	return bundle
 		.generate({
 			format: 'cjs'
 		})
 		.then(({ output: [cjs] }) => {
-			const m = new Function('module', 'exports', cjs.code);
+			const m = new Function('module', 'exports', 'require', cjs.code);
 
 			const module = { exports: {} };
-			m(module, module.exports);
+			m(module, module.exports, require);
 
 			return module.exports;
 		});