Prevent infinite loops when using this.resolve in resolveId

browser/resolveId.ts

@@ -1,21 +1,23 @@
-import { CustomPluginOptions, Plugin } from '../src/rollup/types';
+import { CustomPluginOptions, Plugin, ResolvedId } from '../src/rollup/types';
 import { PluginDriver } from '../src/utils/PluginDriver';
+import { resolveIdViaPlugins } from '../src/utils/resolveIdViaPlugins';
 import { throwNoFileSystem } from './error';
 
 export async function resolveId(
 	source: string,
 	importer: string | undefined,
 	_preserveSymlinks: boolean,
 	pluginDriver: PluginDriver,
-	skip: Plugin | null,
+	moduleLoaderResolveId: (
+		source: string,
+		importer: string | undefined,
+		customOptions: CustomPluginOptions | undefined,
+		skip: { importer: string | undefined; plugin: Plugin; source: string }[] | null
+	) => Promise<ResolvedId | null>,
+	skip: { importer: string | undefined; plugin: Plugin; source: string }[] | null,
 	customOptions: CustomPluginOptions | undefined
 ) {
-	const pluginResult = await pluginDriver.hookFirst(
-		'resolveId',
-		[source, importer, { custom: customOptions }],
-		null,
-		skip
-	);
+	const pluginResult = await resolveIdViaPlugins(source, importer,pluginDriver, moduleLoaderResolveId, skip, customOptions);
 	if (pluginResult == null) {
 		throwNoFileSystem('path.resolve');
 	}

src/ModuleLoader.ts

@@ -136,12 +136,12 @@ export class ModuleLoader {
 		return module;
 	}
 
-	async resolveId(
+	resolveId = async (
 		source: string,
 		importer: string | undefined,
 		customOptions: CustomPluginOptions | undefined,
-		skip: Plugin | null = null
-	): Promise<ResolvedId | null> {
+		skip: { importer: string | undefined; plugin: Plugin; source: string }[] | null = null
+	): Promise<ResolvedId | null> => {
 		return this.addDefaultsToResolvedId(
 			this.getNormalizedResolvedIdWithoutDefaults(
 				this.options.external(source, importer, false)
@@ -151,6 +151,7 @@ export class ModuleLoader {
 							importer,
 							this.options.preserveSymlinks,
 							this.pluginDriver,
+							this.resolveId,
 							skip,
 							customOptions
 					  ),
@@ -445,6 +446,7 @@ export class ModuleLoader {
 			importer,
 			this.options.preserveSymlinks,
 			this.pluginDriver,
+			this.resolveId,
 			null,
 			EMPTY_OBJECT
 		);

src/utils/PluginContext.ts

@@ -153,32 +153,37 @@ export function getPluginContext(
 				yield* moduleIds;
 			}
 
-			const moduleIds = graph.modulesById.keys();
-			return wrappedModuleIds();
-		},
-		parse: graph.contextParse.bind(graph),
-		resolve(source, importer, { custom, skipSelf } = BLANK) {
-			return graph.moduleLoader.resolveId(source, importer, custom, skipSelf ? plugin : null);
-		},
-		resolveId: getDeprecatedContextHandler(
-			(source: string, importer: string | undefined) =>
-				graph.moduleLoader
-					.resolveId(source, importer, BLANK)
-					.then(resolveId => resolveId && resolveId.id),
-			'resolveId',
-			'resolve',
-			plugin.name,
-			true,
-			options
-		),
-		setAssetSource: fileEmitter.setAssetSource,
-		warn(warning) {
-			if (typeof warning === 'string') warning = { message: warning } as RollupWarning;
-			if (warning.code) warning.pluginCode = warning.code;
-			warning.code = 'PLUGIN_WARNING';
-			warning.plugin = plugin.name;
-			options.onwarn(warning);
-		}
-	};
-	return context;
+				const moduleIds = graph.modulesById.keys();
+				return wrappedModuleIds();
+			},
+			parse: graph.contextParse.bind(graph),
+			resolve(source, importer, { custom, skipSelf } = BLANK) {
+				return graph.moduleLoader.resolveId(
+					source,
+					importer,
+					custom,
+					skipSelf ? [{ importer, plugin, source }] : null
+				);
+			},
+			resolveId: getDeprecatedContextHandler(
+				(source: string, importer: string | undefined) =>
+					graph.moduleLoader
+						.resolveId(source, importer, BLANK)
+						.then(resolveId => resolveId && resolveId.id),
+				'resolveId',
+				'resolve',
+				plugin.name,
+				true,
+				options
+			),
+			setAssetSource: fileEmitter.setAssetSource,
+			warn(warning) {
+				if (typeof warning === 'string') warning = { message: warning } as RollupWarning;
+				if (warning.code) warning.pluginCode = warning.code;
+				warning.code = 'PLUGIN_WARNING';
+				warning.plugin = plugin.name;
+				options.onwarn(warning);
+			}
+		};
+		return context;
 }

src/utils/PluginDriver.ts

@@ -59,7 +59,7 @@ const inputHookNames: {
 };
 const inputHooks = Object.keys(inputHookNames);
 
-type ReplaceContext = (context: PluginContext, plugin: Plugin) => PluginContext;
+export type ReplaceContext = (context: PluginContext, plugin: Plugin) => PluginContext;
 
 function throwInvalidHookError(hookName: string, pluginName: string) {
 	return error({
@@ -129,11 +129,11 @@ export class PluginDriver {
 		hookName: H,
 		args: Parameters<PluginHooks[H]>,
 		replaceContext?: ReplaceContext | null,
-		skip?: Plugin | null
+		skipped?: Set<Plugin> | null
 	): EnsurePromise<ReturnType<PluginHooks[H]>> {
 		let promise: EnsurePromise<ReturnType<PluginHooks[H]>> = Promise.resolve(undefined as any);
 		for (const plugin of this.plugins) {
-			if (skip === plugin) continue;
+			if (skipped && skipped.has(plugin)) continue;
 			promise = promise.then(result => {
 				if (result != null) return result;
 				return this.runHook(hookName, args, plugin, false, replaceContext);

src/utils/resolveId.ts

@@ -1,21 +1,30 @@
-import { CustomPluginOptions, Plugin } from '../rollup/types';
+import { CustomPluginOptions, Plugin, ResolvedId } from '../rollup/types';
 import { lstatSync, readdirSync, realpathSync } from './fs';
 import { basename, dirname, isAbsolute, resolve } from './path';
 import { PluginDriver } from './PluginDriver';
+import { resolveIdViaPlugins } from './resolveIdViaPlugins';
 
 export async function resolveId(
 	source: string,
 	importer: string | undefined,
 	preserveSymlinks: boolean,
 	pluginDriver: PluginDriver,
-	skip: Plugin | null,
+	moduleLoaderResolveId: (
+		source: string,
+		importer: string | undefined,
+		customOptions: CustomPluginOptions | undefined,
+		skip: { importer: string | undefined; plugin: Plugin; source: string }[] | null
+	) => Promise<ResolvedId | null>,
+	skip: { importer: string | undefined; plugin: Plugin; source: string }[] | null,
 	customOptions: CustomPluginOptions | undefined
 ) {
-	const pluginResult = await pluginDriver.hookFirst(
-		'resolveId',
-		[source, importer, { custom: customOptions }],
-		null,
-		skip
+	const pluginResult = await resolveIdViaPlugins(
+		source,
+		importer,
+		pluginDriver,
+		moduleLoaderResolveId,
+		skip,
+		customOptions
 	);
 	if (pluginResult != null) return pluginResult;
 

src/utils/resolveIdViaPlugins.ts

@@ -0,0 +1,45 @@
+import { CustomPluginOptions, Plugin, PluginContext, ResolvedId } from '../rollup/types';
+import { BLANK } from './blank';
+import { PluginDriver, ReplaceContext } from './PluginDriver';
+
+export function resolveIdViaPlugins(
+	source: string,
+	importer: string | undefined,
+	pluginDriver: PluginDriver,
+	moduleLoaderResolveId: (
+		source: string,
+		importer: string | undefined,
+		customOptions: CustomPluginOptions | undefined,
+		skip: { importer: string | undefined; plugin: Plugin; source: string }[] | null
+	) => Promise<ResolvedId | null>,
+	skip: { importer: string | undefined; plugin: Plugin; source: string }[] | null,
+	customOptions: CustomPluginOptions | undefined
+) {
+	let skipped: Set<Plugin> | null = null;
+	let replaceContext: ReplaceContext | null = null;
+	if (skip) {
+		skipped = new Set();
+		for (const skippedCall of skip) {
+			if (source === skippedCall.source && importer === skippedCall.importer) {
+				skipped.add(skippedCall.plugin);
+			}
+		}
+		replaceContext = (pluginContext, plugin): PluginContext => ({
+			...pluginContext,
+			resolve: (source, importer, { custom, skipSelf } = BLANK) => {
+				return moduleLoaderResolveId(
+					source,
+					importer,
+					custom,
+					skipSelf ? [...skip, { importer, plugin, source }] : skip
+				);
+			}
+		});
+	}
+	return pluginDriver.hookFirst(
+		'resolveId',
+		[source, importer, { custom: customOptions }],
+		replaceContext,
+		skipped
+	);
+}

test/function/samples/prevent-context-resolve-loop/_config.js

@@ -0,0 +1,31 @@
+const path = require('path');
+
+const ID_OTHER_1 = path.join(__dirname, 'other1.js');
+const ID_OTHER_2 = path.join(__dirname, 'other2.js');
+const ID_OTHER_3 = path.join(__dirname, 'other3.js');
+
+module.exports = {
+	description: 'prevents infinite loops when several plugins are calling this.resolve in resolveId',
+	options: {
+		plugins: [
+			{
+				name: 'first',
+				async resolveId(source, importer) {
+					const { id } = await this.resolve(source, importer, { skipSelf: true });
+					if (id === ID_OTHER_2) {
+						return ID_OTHER_3;
+					}
+				}
+			},
+			{
+				name: 'second',
+				async resolveId(source, importer) {
+					const { id } = await this.resolve(source, importer, { skipSelf: true });
+					if (id === ID_OTHER_1) {
+						return ID_OTHER_2;
+					}
+				}
+			}
+		]
+	}
+};

test/function/samples/prevent-context-resolve-loop/main.js

@@ -0,0 +1,5 @@
+import first from './other1.js';
+import second from './other2.js';
+
+assert.strictEqual(first, 3);
+assert.strictEqual(second, 3);

test/function/samples/prevent-context-resolve-loop/other1.js

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

test/function/samples/prevent-context-resolve-loop/other2.js

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

test/function/samples/prevent-context-resolve-loop/other3.js

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