Do not resolve relative external ids when using the object form (#2907)

* Do not renormalize external ids when using the object form

* Update documentation

docs/05-plugins.md

@@ -231,19 +231,21 @@ resolveFileUrl({fileName}) {
 Type: `(source: string, importer: string) => string | false | null | {id: string, external?: boolean, moduleSideEffects?: boolean | null}`<br>
 Kind: `async, first`
 
-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 `source` 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 `source` should be treated as an external module and not included in the bundle. If this happens for a relative import, the id will be renormalized the same way as when the `external` option is used.
 
 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(source) {
   if (source === 'my-dependency') {
-    return {source: 'my-dependency-develop', external: true};
+    return {id: 'my-dependency-develop', external: true};
   }
   return null;
 }
 ```
 
+Relative ids, i.e. starting with `./` or `../`, will **not** be renormalized when returning an object. If you want this behaviour, return an absolute file system location as `id` instead.
+
 If `false` is returned for `moduleSideEffects` in the first hook that resolves a module id and no other module imports anything from this module, then this module will not be included without checking for actual side-effects inside the module. If `true` is returned, Rollup will use its default algorithm to include all statements in the module that have side-effects (such as modifying a global or exported variable). If `null` is returned or the flag is omitted, then `moduleSideEffects` will be determined by the `treeshake.moduleSideEffects` option or default to `true`. The `load` and `transform` hooks can override this.
 
 #### `resolveImportMeta`

docs/999-big-list-of-options.md

@@ -40,6 +40,31 @@ When providing a function, it is actually called with three parameters `(id, par
 
 When creating an `iife` or `umd` bundle, you will need to provide global variable names to replace your external imports via the `output.globals` option.
 
+If a relative import, i.e. starting with `./` or `../`, is marked as "external", rollup will internally resolve the id to an absolute file system location so that different imports of the external module can be merged. When the resulting bundle is written, the import will again be converted to a relative import. Example:
+
+```js
+// input
+// src/main.js (entry point)
+import x from '../external.js';
+import './nested/nested.js';
+console.log(x);
+
+// src/nested/nested.js
+// the import would point to the same file if it existed
+import x from '../../external.js';
+console.log(x);
+
+// output
+// the different imports are merged
+import x from '../external.js';
+
+console.log(x);
+
+console.log(x);
+```
+
+The conversion back to a relative import is done as if `output.file` or `output.dir` were in the same location as the entry point or the common base directory of all entry points if there is more than one.
+
 #### input
 Type: `string | string [] | { [entryName: string]: string }`<br>
 CLI: `-i`/`--input <filename>`

src/ModuleLoader.ts

@@ -216,12 +216,18 @@ export class ModuleLoader {
 		return fileName;
 	}
 
-	resolveId(source: string, importer: string, skip?: number | null): Promise<ResolvedId | null> {
-		return Promise.resolve(
+	async resolveId(
+		source: string,
+		importer: string,
+		skip?: number | null
+	): Promise<ResolvedId | null> {
+		return this.normalizeResolveIdResult(
 			this.isExternal(source, importer, false)
-				? { id: source, external: true }
-				: this.pluginDriver.hookFirst('resolveId', [source, importer], null, skip as number)
-		).then(result => this.normalizeResolveIdResult(result, importer, source));
+				? false
+				: await this.pluginDriver.hookFirst('resolveId', [source, importer], null, skip),
+			importer,
+			source
+		);
 	}
 
 	private addToManualChunk(alias: string, module: Module) {
@@ -455,13 +461,10 @@ export class ModuleLoader {
 					moduleSideEffects = resolveIdResult.moduleSideEffects;
 				}
 			} else {
-				id = resolveIdResult;
-				if (this.isExternal(id, importer, true)) {
+				if (this.isExternal(resolveIdResult, importer, true)) {
 					external = true;
 				}
-			}
-			if (external) {
-				id = normalizeRelativeExternalId(importer, id);
+				id = external ? normalizeRelativeExternalId(importer, resolveIdResult) : resolveIdResult;
 			}
 		} else {
 			id = normalizeRelativeExternalId(importer, source);
@@ -480,19 +483,15 @@ export class ModuleLoader {
 		};
 	}
 
-	private resolveAndFetchDependency(
+	private async resolveAndFetchDependency(
 		module: Module,
 		source: string
 	): Promise<Module | ExternalModule> {
-		return Promise.resolve(
+		const resolvedId =
 			module.resolvedIds[source] ||
-				this.resolveId(source, module.id).then(resolvedId =>
-					this.handleMissingImports(resolvedId, source, module.id)
-				)
-		).then(resolvedId => {
-			module.resolvedIds[source] = resolvedId;
-			return this.fetchResolvedDependency(source, module.id, resolvedId);
-		});
+			this.handleMissingImports(await this.resolveId(source, module.id), source, module.id);
+		module.resolvedIds[source] = resolvedId;
+		return this.fetchResolvedDependency(source, module.id, resolvedId);
 	}
 
 	private resolveDynamicImport(

src/utils/pluginDriver.ts

@@ -34,7 +34,7 @@ export interface PluginDriver {
 		hook: H,
 		args: Args<PluginHooks[H]>,
 		hookContext?: HookContext | null,
-		skip?: number
+		skip?: number | null
 	): EnsurePromise<R>;
 	hookFirstSync<H extends keyof PluginHooks, R = ReturnType<PluginHooks[H]>>(
 		hook: H,

test/form/samples/relative-external-ids/_expected.js

@@ -5,4 +5,4 @@ import './resolved.js';
 import './nested/optionDirectNested.js';
 import './nested/optionIndirectNested.js';
 import './nested/hookNested.js';
-import './nested/resolvedNested.js';
+import './resolvedNested.js';