Allow resolveId to return an object (#2734)

* Allow resolveId to return an object

* Improve wording and type in docs

docs/05-plugins.md

@@ -180,10 +180,21 @@ Kind: `async, first`
 Defines a custom resolver for dynamic imports. In case a dynamic import is not passed a string as argument, this hook gets access to the raw AST nodes to analyze. Returning `null` will defer to other resolvers and eventually to `resolveId` if this is possible; returning `false` signals that the import should be kept as it is and not be passed to other resolvers thus making it external. Note that the return value of this hook will not be passed to `resolveId` afterwards; if you need access to the static resolution algorithm, you can use `this.resolveId(importee, importer)` on the plugin context.
 
 #### `resolveId`
-Type: `(importee: string, importer: string) => string | false | null`<br>
+Type: `(importee: string, importer: string) => string | false | null | {id: string, external?: boolean}`<br>
 Kind: `async, first`
 
-Defines a custom resolver. A resolver loader can be useful for e.g. locating third-party dependencies. Returning `null` defers to other `resolveId` functions (and eventually the default resolution behavior); returning `false` signals that `importee` should be treated as an external module and not included in the bundle.
+Defines a custom resolver. A resolver can be useful for e.g. locating third-party dependencies. Returning `null` defers to other `resolveId` functions (and eventually the default resolution behavior); returning `false` signals that `importee` should be treated as an external module and not included in the bundle.
+
+If you return an object, then it is possible to resolve an import to a different id while excluding it from the bundle at the same time. This allows you to replace dependencies with external dependencies without the need for the user to mark them as "external" manually via the `external` option:
+
+```js
+resolveId(id) {
+	if (id === 'my-dependency') {
+		return {id: 'my-dependency-develop', external: true};
+	}
+	return null;
+}
+```
 
 #### `transform`
 Type: `(code: string, id: string) => string | { code: string, map?: string | SourceMap, ast? : ESTree.Program } | null`

src/Graph.ts

@@ -13,6 +13,8 @@ import {
 	IsExternal,
 	ModuleJSON,
 	OutputBundle,
+	ResolvedId,
+	ResolveIdResult,
 	RollupCache,
 	RollupWarning,
 	RollupWatcher,
@@ -437,81 +439,7 @@ export default class Graph {
 		fetchDynamicImportsPromise.catch(() => {});
 
 		return Promise.all(
-			module.sources.map(source => {
-				return Promise.resolve()
-					.then(() => {
-						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,
-							module.id
-						]);
-					})
-					.then(resolvedId => {
-						// TODO types of `resolvedId` are not compatible with 'externalId'.
-						// `this.resolveId` returns `string`, `void`, and `boolean`
-						const externalId =
-							<string>resolvedId ||
-							(isRelative(source) ? resolve(module.id, '..', source) : source);
-						let isExternal = resolvedId === false || this.isExternal(externalId, module.id, true);
-
-						if (!resolvedId && !isExternal) {
-							if (isRelative(source)) {
-								error({
-									code: 'UNRESOLVED_IMPORT',
-									message: `Could not resolve '${source}' from ${relativeId(module.id)}`
-								});
-							}
-
-							if (resolvedId !== false) {
-								this.warn({
-									code: 'UNRESOLVED_IMPORT',
-									importer: relativeId(module.id),
-									message: `'${source}' is imported by ${relativeId(
-										module.id
-									)}, but could not be resolved – treating it as an external dependency`,
-									source,
-									url:
-										'https://rollupjs.org/guide/en#warning-treating-module-as-external-dependency'
-								});
-							}
-							isExternal = true;
-						}
-
-						if (isExternal) {
-							module.resolvedIds[source] = { id: externalId, external: true };
-
-							if (!this.moduleById.has(externalId)) {
-								const module = new ExternalModule({ graph: this, id: externalId });
-								this.externalModules.push(module);
-								this.moduleById.set(externalId, module);
-							}
-
-							const externalModule = this.moduleById.get(externalId);
-
-							if (externalModule instanceof ExternalModule === false) {
-								error({
-									code: 'INVALID_EXTERNAL_ID',
-									message: `'${source}' is imported as an external by ${relativeId(
-										module.id
-									)}, but is already an existing non-external module id.`
-								});
-							}
-
-							for (const name in module.importDescriptions) {
-								const importDeclaration = module.importDescriptions[name];
-								if (importDeclaration.source !== source) return;
-
-								// this will trigger a warning for unused external imports
-								externalModule.getVariableForExportName(importDeclaration.name);
-							}
-						} else {
-							module.resolvedIds[source] = { id: <string>resolvedId, external: false };
-							return this.fetchModule(<string>resolvedId, module.id);
-						}
-					});
-			})
+			module.sources.map(source => this.resolveAndFetchDependency(module, source))
 		).then(() => fetchDynamicImportsPromise);
 	}
 
@@ -703,6 +631,70 @@ export default class Graph {
 			});
 	}
 
+	private normalizeResolveIdResult(
+		resolveIdResult: ResolveIdResult,
+		module: Module,
+		source: string
+	): ResolvedId {
+		if (resolveIdResult) {
+			if (typeof resolveIdResult === 'object') {
+				return resolveIdResult;
+			}
+			return { id: resolveIdResult, external: this.isExternal(resolveIdResult, module.id, true) };
+		}
+		const externalId = isRelative(source) ? resolve(module.id, '..', source) : source;
+		if (resolveIdResult !== false && !this.isExternal(externalId, module.id, true)) {
+			if (isRelative(source)) {
+				error({
+					code: 'UNRESOLVED_IMPORT',
+					message: `Could not resolve '${source}' from ${relativeId(module.id)}`
+				});
+			}
+			this.warn({
+				code: 'UNRESOLVED_IMPORT',
+				importer: relativeId(module.id),
+				message: `'${source}' is imported by ${relativeId(
+					module.id
+				)}, but could not be resolved – treating it as an external dependency`,
+				source,
+				url: 'https://rollupjs.org/guide/en#warning-treating-module-as-external-dependency'
+			});
+		}
+		return { id: externalId, external: true };
+	}
+
+	private resolveAndFetchDependency(module: Module, source: string) {
+		return Promise.resolve(
+			module.resolvedIds[source] ||
+				(this.isExternal(source, module.id, false)
+					? { id: source, external: true }
+					: this.pluginDriver
+							.hookFirst<ResolveIdResult>('resolveId', [source, module.id])
+							.then(result => this.normalizeResolveIdResult(result, module, source)))
+		).then((resolvedId: ResolvedId) => {
+			module.resolvedIds[source] = resolvedId;
+			if (resolvedId.external) {
+				if (!this.moduleById.has(resolvedId.id)) {
+					const module = new ExternalModule({ graph: this, id: resolvedId.id });
+					this.externalModules.push(module);
+					this.moduleById.set(resolvedId.id, module);
+				}
+
+				const externalModule = this.moduleById.get(resolvedId.id);
+				if (externalModule instanceof ExternalModule === false) {
+					error({
+						code: 'INVALID_EXTERNAL_ID',
+						message: `'${source}' is imported as an external by ${relativeId(
+							module.id
+						)}, but is already an existing non-external module id.`
+					});
+				}
+			} else {
+				return this.fetchModule(resolvedId.id, module.id);
+			}
+		});
+	}
+
 	private warnForMissingExports() {
 		for (const module of this.modules) {
 			for (const importName of Object.keys(module.importDescriptions)) {

src/Module.ts

@@ -28,7 +28,14 @@ import Variable from './ast/variables/Variable';
 import Chunk from './Chunk';
 import ExternalModule from './ExternalModule';
 import Graph from './Graph';
-import { Asset, IdMap, ModuleJSON, RawSourceMap, RollupError, RollupWarning } from './rollup/types';
+import {
+	Asset,
+	ModuleJSON,
+	RawSourceMap,
+	ResolvedIdMap,
+	RollupError,
+	RollupWarning
+} from './rollup/types';
 import { error } from './utils/error';
 import getCodeFrame from './utils/getCodeFrame';
 import { getOriginalLocation } from './utils/getOriginalLocation';
@@ -187,7 +194,7 @@ export default class Module {
 	originalCode: string;
 	originalSourcemap: RawSourceMap | void;
 	reexports: { [name: string]: ReexportDescription } = Object.create(null);
-	resolvedIds: IdMap;
+	resolvedIds: ResolvedIdMap;
 	scope: ModuleScope;
 	sourcemapChain: RawSourceMap[];
 	sources: string[] = [];

src/rollup/types.d.ts

@@ -3,10 +3,6 @@ import { EventEmitter } from 'events';
 
 export const VERSION: string;
 
-export interface IdMap {
-	[key: string]: { external: boolean; id: string };
-}
-
 export interface RollupError extends RollupLogProps {
 	stack?: string;
 }
@@ -84,7 +80,7 @@ export interface ModuleJSON {
 	id: string;
 	originalCode: string;
 	originalSourcemap: RawSourceMap | void;
-	resolvedIds: IdMap;
+	resolvedIds: ResolvedIdMap;
 	sourcemapChain: RawSourceMap[];
 	transformAssets: Asset[] | void;
 	transformDependencies: string[] | null;
@@ -134,11 +130,22 @@ export interface PluginContextMeta {
 	rollupVersion: string;
 }
 
+export interface ResolvedId {
+	external?: boolean | void;
+	id: string;
+}
+
+export type ResolveIdResult = string | false | void | ResolvedId;
+
+export interface ResolvedIdMap {
+	[key: string]: ResolvedId;
+}
+
 export type ResolveIdHook = (
 	this: PluginContext,
 	id: string,
 	parent: string
-) => Promise<string | false | null> | string | false | void | null;
+) => Promise<ResolveIdResult> | ResolveIdResult;
 
 export type IsExternal = (id: string, parentId: string, isResolved: boolean) => boolean | void;
 

test/function/samples/resolve-id-object/_config.js

@@ -0,0 +1,28 @@
+const path = require('path');
+
+module.exports = {
+	description: 'allows resolving an id with an object',
+	options: {
+		plugins: {
+			resolveId(importee) {
+				const fooId = path.resolve(__dirname, 'foo.js');
+				switch (importee) {
+					case 'internal1':
+						return { id: fooId };
+					case 'internal2':
+						return { id: fooId, external: false };
+					case 'external':
+						return { id: 'my-external', external: true };
+				}
+			}
+		}
+	},
+	context: {
+		require(id) {
+			if (id === 'my-external') {
+				return 'external';
+			}
+			throw new Error(`Unexpected external id ${id}.`);
+		}
+	}
+};

test/function/samples/resolve-id-object/foo.js

@@ -0,0 +1 @@
+export default 42;

test/function/samples/resolve-id-object/main.js

@@ -0,0 +1,9 @@
+import external from 'external';
+import internal1 from 'internal1';
+import internal2 from 'internal2';
+
+assert.strictEqual(internal1, 42);
+
+assert.strictEqual(internal2, 42);
+
+assert.strictEqual(external, 'external');

test/function/samples/unused-import/_config.js

@@ -1,13 +1,14 @@
 module.exports = {
 	description: 'warns on unused imports ([#595])',
+	options: {
+		external: ['external']
+	},
+	context: {
+		require(id) {
+			return {};
+		}
+	},
 	warnings: [
-		{
-			code: 'UNRESOLVED_IMPORT',
-			importer: 'main.js',
-			source: 'external',
-			message: `'external' is imported by main.js, but could not be resolved – treating it as an external dependency`,
-			url: `https://rollupjs.org/guide/en#warning-treating-module-as-external-dependency`
-		},
 		{
 			code: 'UNUSED_EXTERNAL_IMPORT',
 			source: 'external',

test/misc/misc.js

@@ -34,15 +34,16 @@ describe('misc', () => {
 			});
 	});
 
-	it('warns when globals option is specified and a global module name is guessed in a UMD bundle (#2358)', () => {
+	it('warns when a global module name is guessed in a UMD bundle (#2358)', () => {
 		const warnings = [];
 
 		return rollup
 			.rollup({
 				input: 'input',
+				external: ['lodash'],
 				plugins: [
 					loader({
-						input: `import * as _ from 'lodash'`
+						input: `import * as _ from 'lodash'; console.log(_);`
 					})
 				],
 				onwarn: warning => warnings.push(warning)
@@ -55,12 +56,16 @@ describe('misc', () => {
 				})
 			)
 			.then(() => {
-				const relevantWarnings = warnings.filter(warning => warning.code === 'MISSING_GLOBAL_NAME');
-				assert.equal(relevantWarnings.length, 1);
-				assert.equal(
-					relevantWarnings[0].message,
-					`No name was provided for external module 'lodash' in output.globals – guessing 'lodash'`
-				);
+				delete warnings[0].toString;
+				assert.deepEqual(warnings, [
+					{
+						code: 'MISSING_GLOBAL_NAME',
+						guess: '_',
+						message:
+							"No name was provided for external module 'lodash' in output.globals – guessing '_'",
+						source: 'lodash'
+					}
+				]);
 			});
 	});
 
@@ -110,15 +115,23 @@ describe('misc', () => {
 				Promise.all([
 					bundle
 						.generate({ format: 'esm' })
-						.then(generated => assert.equal(generated.output[0].code, "import 'the-answer';\n", 'no render path 1')),
+						.then(generated =>
+							assert.equal(generated.output[0].code, "import 'the-answer';\n", 'no render path 1')
+						),
 					bundle
 						.generate({ format: 'esm', paths: id => `//unpkg.com/${id}@?module` })
 						.then(generated =>
-							assert.equal(generated.output[0].code, "import '//unpkg.com/the-answer@?module';\n", 'with render path')
+							assert.equal(
+								generated.output[0].code,
+								"import '//unpkg.com/the-answer@?module';\n",
+								'with render path'
+							)
 						),
 					bundle
 						.generate({ format: 'esm' })
-						.then(generated => assert.equal(generated.output[0].code, "import 'the-answer';\n", 'no render path 2'))
+						.then(generated =>
+							assert.equal(generated.output[0].code, "import 'the-answer';\n", 'no render path 2')
+						)
 				])
 			);
 	});