From 2cf6f68dbcffb014b1fd55f0acfb88950097348b Mon Sep 17 00:00:00 2001 From: "andreas.karlsson" Date: Tue, 26 Feb 2019 07:58:33 +0100 Subject: [PATCH] Incremental external (#2718) * Added failing test for incremental build with externals * Let resolvedIds track both id and whether external --- src/Graph.ts | 10 +++++----- src/Module.ts | 7 ++++--- src/rollup/types.d.ts | 2 +- src/utils/pluginDriver.ts | 2 +- test/incremental/index.js | 42 ++++++++++++++++++++++++++++++++++++--- test/utils.js | 6 +++--- 6 files changed, 53 insertions(+), 16 deletions(-) diff --git a/src/Graph.ts b/src/Graph.ts index 542db768fb9..e369042092d 100644 --- a/src/Graph.ts +++ b/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('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] = resolvedId; + module.resolvedIds[source] = { id: resolvedId, external: false }; return this.fetchModule(resolvedId, module.id); } }); diff --git a/src/Module.ts b/src/Module.ts index 775353fa1ef..a7827e92dc5 100644 --- a/src/Module.ts +++ b/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); } } diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts index 69e3db5c81f..58e7830d020 100644 --- a/src/rollup/types.d.ts +++ b/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 { diff --git a/src/utils/pluginDriver.ts b/src/utils/pluginDriver.ts index d99534c98a8..16bf33700a9 100644 --- a/src/utils/pluginDriver.ts +++ b/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 diff --git a/test/incremental/index.js b/test/incremental/index.js index 7d0e6adf77b..9ed2f207e42 100644 --- a/test/incremental/index.js +++ b/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 } }); }); }); diff --git a/test/utils.js b/test/utils.js index bd9ab7b32bc..d21f62e33d9 100644 --- a/test/utils.js +++ b/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; });