Improve absolute path handling

docs/05-plugin-development.md

@@ -157,7 +157,7 @@ In case a dynamic import is not passed a string as argument, this hook gets acce
 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.resolve(source, importer)`](guide/en/#thisresolvesource-string-importer-string-options-skipself-boolean-custom-plugin-string-any--promiseid-string-external-boolean-modulesideeffects-boolean--no-treeshake-syntheticnamedexports-boolean--string-meta-plugin-string-any--null) on the plugin context.
 
 #### `resolveId`
-Type: `(source: string, importer: string | undefined, options: {custom?: {[plugin: string]: any}) => string | false | null | {id: string, external?: boolean, moduleSideEffects?: boolean | "no-treeshake" | null, syntheticNamedExports?: boolean | string | null, meta?: {[plugin: string]: any} | null}`<br>
+Type: `(source: string, importer: string | undefined, options: {custom?: {[plugin: string]: any}) => string | false | null | {id: string, external?: boolean | "relative" | "absolute", moduleSideEffects?: boolean | "no-treeshake" | null, syntheticNamedExports?: boolean | string | null, meta?: {[plugin: string]: any} | null}`<br>
 Kind: `async, first`<br>
 Previous Hook: [`buildStart`](guide/en/#buildstart) if we are resolving an entry point, [`moduleParsed`](guide/en/#moduleparsed) if we are resolving an import, or as fallback for [`resolveDynamicImport`](guide/en/#resolvedynamicimport). Additionally this hook can be triggered during the build phase from plugin hooks by calling [`this.emitFile`](guide/en/#thisemitfileemittedfile-emittedchunk--emittedasset--string) to emit an entry point or at any time by calling [`this.resolve`](guide/en/#thisresolvesource-string-importer-string-options-skipself-boolean-custom-plugin-string-any--promiseid-string-external-boolean-modulesideeffects-boolean--no-treeshake-syntheticnamedexports-boolean--string-meta-plugin-string-any--null) to manually resolve an id.<br>
 Next Hook: [`load`](guide/en/#load) if the resolved id that has not yet been loaded, otherwise [`buildEnd`](guide/en/#buildend).
@@ -208,7 +208,7 @@ resolveId(source) {
 }
 ```
 
-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 `external` is `true`, then absolute ids will be converted to relative ids based on the user's choice for the [`makeAbsoluteExternalsRelative`](guide/en/#makeabsoluteexternalsrelative) option. This choice can be overridden by passing either `external: "relative"` to always convert an absolute id to a relative id or `external: "absolute"` to keep it as an absolute id. When returning an object, relative external ids, i.e. ids starting with `./` or `../`, will *not* be internally converted to an absolute id and converted back to a relative id in the output, but are instead included in the output unchanged. If you want relative ids to be renormalised and deduplicated instead, return an absolute file system location as `id` and choose `external: "relative"`.
 
 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 even if the module would have side-effects. 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 `"no-treeshake"` is returned, treeshaking will be turned off for this module and it will also be included in one of the generated chunks even if it is empty. 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.
 
@@ -695,8 +695,8 @@ An object containing potentially useful Rollup metadata:
 
 Use Rollup's internal acorn instance to parse code to an AST.
 
-#### `this.resolve(source: string, importer?: string, options?: {skipSelf?: boolean, custom?: {[plugin: string]: any}}) => Promise<{id: string, external: boolean, moduleSideEffects: boolean | 'no-treeshake', syntheticNamedExports: boolean | string, meta: {[plugin: string]: any}} | null>`
-Resolve imports to module ids (i.e. file names) using the same plugins that Rollup uses, and determine if an import should be external. If `null` is returned, the import could not be resolved by Rollup or any plugin but was not explicitly marked as external by the user.
+#### `this.resolve(source: string, importer?: string, options?: {skipSelf?: boolean, custom?: {[plugin: string]: any}}) => Promise<{id: string, external: boolean | "absolute", moduleSideEffects: boolean | 'no-treeshake', syntheticNamedExports: boolean | string, meta: {[plugin: string]: any}} | null>`
+Resolve imports to module ids (i.e. file names) using the same plugins that Rollup uses, and determine if an import should be external. If `null` is returned, the import could not be resolved by Rollup or any plugin but was not explicitly marked as external by the user. If an absolute external id is returned that should remain absolute in the output either via the [`makeAbsoluteExternalsRelative`](guide/en/#makeabsoluteexternalsrelative) option or by explicit plugin choice in the [`resolveId`](guide/en/#resolveid) hook, `external` will be `"absolute"` instead of `true`.
 
 If you pass `skipSelf: true`, then the `resolveId` hook of the plugin from which `this.resolve` is called will be skipped when resolving. When other plugins themselves also call `this.resolve` in their `resolveId` hooks with the *exact same `source` and `importer`* while handling the original `this.resolve` call, then the `resolveId` hook of the original plugin will be skipped for those calls as well. The rationale here is that the plugin already stated that it "does not know" how to resolve this particular combination of `source` and `importer` at this point in time. If you do not want this behaviour, do not use `skipSelf` but implement your own infinite loop prevention mechanism if necessary.
 

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

@@ -319,6 +319,23 @@ buildWithCache()
   })
 ```
 
+#### makeAbsoluteExternalsRelative
+Type: `boolean | "ifRelativeSource"`<br>
+CLI: `--makeAbsoluteExternalsRelative`/`--no-makeAbsoluteExternalsRelative`<br>
+Default: `true`
+
+Determines if absolute external paths should be converted to relative paths in the output. This does not only apply to paths that are absolute in the source but also to paths that are resolved to an absolute path by either a plugin or Rollup core.
+
+For `true`, an external import like `import "/Users/Rollup/project/relative.js"` would be converted to a relative path. When converting an absolute path to a relative path, Rollup does *not* take the `file` or `dir` options into account, because those may not be present e.g. for builds using the JavaScript API. Instead, it assumes that the root of the generated bundle is located at the common shared parent directory of all modules that were included in the bundle. Assuming that the common parent directory of all modules is `"/Users/Rollup/project"`, the import from above would likely be converted to `import "./relative.js"` in the output. If the output chunk is itself nested in a sub-directory by choosing e.g. `chunkFileNames: "chunks/[name].js"`, the import would be `"../relative.js"`.
+
+As stated before, this would also apply to originally relative imports like `import "./relative.js"` that are resolved to an absolute path before they are marked as external by the [`external`](guide/en/#external) option.
+
+One common problem is that this mechanism will also apply to imports like `import "/absolute.js'"`, resulting in unexpected relative paths in the output.
+
+For this case, choosing `"ifRelativeSource"` will check if the original import was a relative import and only then convert it to a relative import in the output. Choosing `false` will keep all paths as absolute paths in the output.
+
+Note that when a relative path is directly marked as "external" using the [`external`](guide/en/#external) option, then it will be the same relative path in the output. When it is resolved first via a plugin or Rollup core and then marked as external, the above logic will apply.
+
 #### onwarn
 Type: `(warning: RollupWarning, defaultHandler: (warning: string | RollupWarning) => void) => void;`
 
@@ -360,7 +377,6 @@ export default {
 };
 ```
 
-
 #### output.assetFileNames
 Type: `string | ((assetInfo: AssetInfo) => string)`<br>
 CLI: `--assetFileNames <pattern>`<br>

src/Chunk.ts

@@ -1250,7 +1250,11 @@ export default class Chunk {
 		}
 	}
 
-	private setIdentifierRenderResolutions({ format, interop, namespaceToStringTag }: NormalizedOutputOptions) {
+	private setIdentifierRenderResolutions({
+		format,
+		interop,
+		namespaceToStringTag
+	}: NormalizedOutputOptions) {
 		const syntheticExports = new Set<SyntheticNamedExportVariable>();
 		for (const exportName of this.getExportNames()) {
 			const exportVariable = this.exportsByName[exportName];

src/ExternalModule.ts

@@ -7,7 +7,7 @@ import {
 } from './rollup/types';
 import { EMPTY_ARRAY } from './utils/blank';
 import { makeLegal } from './utils/identifierHelpers';
-import { isAbsolute, normalize, relative } from './utils/path';
+import { normalize, relative } from './utils/path';
 
 export default class ExternalModule {
 	chunk: void;
@@ -23,7 +23,6 @@ export default class ExternalModule {
 	nameSuggestions: { [name: string]: number };
 	reexported = false;
 	renderPath: string = undefined as any;
-	renormalizeRenderPath = false;
 	suggestedVariableName: string;
 	used = false;
 	variableName = '';
@@ -32,7 +31,8 @@ export default class ExternalModule {
 		private readonly options: NormalizedInputOptions,
 		public readonly id: string,
 		hasModuleSideEffects: boolean | 'no-treeshake',
-		meta: CustomPluginOptions
+		meta: CustomPluginOptions,
+		public renormalizeRenderPath: boolean
 	) {
 		this.execIndex = Infinity;
 		this.suggestedVariableName = makeLegal(id.split(/[\\/]/).pop()!);
@@ -76,12 +76,9 @@ export default class ExternalModule {
 		this.renderPath =
 			typeof options.paths === 'function' ? options.paths(this.id) : options.paths[this.id];
 		if (!this.renderPath) {
-			if (!isAbsolute(this.id)) {
-				this.renderPath = this.id;
-			} else {
-				this.renderPath = normalize(relative(inputBase, this.id));
-				this.renormalizeRenderPath = true;
-			}
+			this.renderPath = this.renormalizeRenderPath
+				? normalize(relative(inputBase, this.id))
+				: this.id;
 		}
 		return this.renderPath;
 	}

src/ModuleLoader.ts

@@ -6,8 +6,9 @@ import {
 	CustomPluginOptions,
 	EmittedChunk,
 	HasModuleSideEffects,
+	ModuleOptions,
 	NormalizedInputOptions,
-	PartialResolvedId,
+	PartialNull,
 	Plugin,
 	ResolvedId,
 	ResolveIdResult,
@@ -27,7 +28,7 @@ import {
 	errUnresolvedImportTreatedAsExternal
 } from './utils/error';
 import { readFile } from './utils/fs';
-import { isRelative, resolve } from './utils/path';
+import { isAbsolute, isRelative, resolve } from './utils/path';
 import { PluginDriver } from './utils/PluginDriver';
 import relativeId from './utils/relativeId';
 import { resolveId } from './utils/resolveId';
@@ -41,6 +42,11 @@ export interface UnresolvedModule {
 	name: string | null;
 }
 
+type NormalizedResolveIdWithoutDefaults = Partial<PartialNull<ModuleOptions>> & {
+	external?: boolean | 'absolute';
+	id: string;
+};
+
 export class ModuleLoader {
 	private readonly hasModuleSideEffects: HasModuleSideEffects;
 	private readonly implicitEntryModules = new Set<Module>();
@@ -160,9 +166,11 @@ export class ModuleLoader {
 				source
 			)
 		);
-	}
+	};
 
-	private addDefaultsToResolvedId(resolvedId: PartialResolvedId | null): ResolvedId | null {
+	private addDefaultsToResolvedId(
+		resolvedId: NormalizedResolveIdWithoutDefaults | null
+	): ResolvedId | null {
 		if (!resolvedId) {
 			return null;
 		}
@@ -172,7 +180,7 @@ export class ModuleLoader {
 			id: resolvedId.id,
 			meta: resolvedId.meta || EMPTY_OBJECT,
 			moduleSideEffects:
-				resolvedId.moduleSideEffects ?? this.hasModuleSideEffects(resolvedId.id, external),
+				resolvedId.moduleSideEffects ?? this.hasModuleSideEffects(resolvedId.id, !!external),
 			syntheticNamedExports: resolvedId.syntheticNamedExports ?? false
 		};
 	}
@@ -338,19 +346,21 @@ export class ModuleLoader {
 		resolvedId: ResolvedId
 	): Promise<Module | ExternalModule> {
 		if (resolvedId.external) {
-			if (!this.modulesById.has(resolvedId.id)) {
+			const { external, id, moduleSideEffects, meta } = resolvedId;
+			if (!this.modulesById.has(id)) {
 				this.modulesById.set(
-					resolvedId.id,
+					id,
 					new ExternalModule(
 						this.options,
-						resolvedId.id,
-						resolvedId.moduleSideEffects,
-						resolvedId.meta
+						id,
+						moduleSideEffects,
+						meta,
+						external !== 'absolute' && isAbsolute(id)
 					)
 				);
 			}
 
-			const externalModule = this.modulesById.get(resolvedId.id);
+			const externalModule = this.modulesById.get(id);
 			if (!(externalModule instanceof ExternalModule)) {
 				return error(errInternalIdCannotBeExternal(source, importer));
 			}
@@ -385,27 +395,45 @@ export class ModuleLoader {
 		resolveIdResult: ResolveIdResult,
 		importer: string | undefined,
 		source: string
-	): (PartialResolvedId & { external: boolean }) | null {
+	): NormalizedResolveIdWithoutDefaults | null {
+		const { makeAbsoluteExternalsRelative } = this.options;
 		if (resolveIdResult) {
 			if (typeof resolveIdResult === 'object') {
+				const external =
+					resolveIdResult.external || this.options.external(resolveIdResult.id, importer, true);
 				return {
 					...resolveIdResult,
 					external:
-						resolveIdResult.external || this.options.external(resolveIdResult.id, importer, true)
+						external &&
+						(external === 'relative' ||
+							!isAbsolute(resolveIdResult.id) ||
+							(external === true &&
+								isNotAbsoluteExternal(resolveIdResult.id, source, makeAbsoluteExternalsRelative)) ||
+							'absolute')
 				};
 			}
+
 			const external = this.options.external(resolveIdResult, importer, true);
 			return {
-				external,
-				id: external ? normalizeRelativeExternalId(resolveIdResult, importer) : resolveIdResult
+				external:
+					external &&
+					(isNotAbsoluteExternal(resolveIdResult, source, makeAbsoluteExternalsRelative) ||
+						'absolute'),
+				id:
+					external && makeAbsoluteExternalsRelative
+						? normalizeRelativeExternalId(resolveIdResult, importer)
+						: resolveIdResult
 			};
 		}
-		const id = normalizeRelativeExternalId(source, importer);
+
+		const id = makeAbsoluteExternalsRelative
+			? normalizeRelativeExternalId(source, importer)
+			: source;
 		if (resolveIdResult !== false && !this.options.external(id, importer, true)) {
 			return null;
 		}
 		return {
-			external: true,
+			external: isNotAbsoluteExternal(id, source, makeAbsoluteExternalsRelative) || 'absolute',
 			id
 		};
 	}
@@ -469,7 +497,9 @@ export class ModuleLoader {
 		}
 		return this.fetchModule(
 			this.addDefaultsToResolvedId(
-				typeof resolveIdResult === 'object' ? resolveIdResult : { id: resolveIdResult }
+				typeof resolveIdResult === 'object'
+					? (resolveIdResult as NormalizedResolveIdWithoutDefaults)
+					: { id: resolveIdResult }
 			)!,
 			undefined,
 			isEntry
@@ -541,3 +571,15 @@ function addChunkNamesToModule(
 		}
 	}
 }
+
+function isNotAbsoluteExternal(
+	id: string,
+	source: string,
+	makeAbsoluteExternalsRelative: boolean | 'ifRelativeSource'
+) {
+	return (
+		makeAbsoluteExternalsRelative === true ||
+		(makeAbsoluteExternalsRelative === 'ifRelativeSource' && isRelative(source)) ||
+		!isAbsolute(id)
+	);
+}

src/rollup/types.d.ts

@@ -219,7 +219,7 @@ export interface PluginContextMeta {
 }
 
 export interface ResolvedId extends ModuleOptions {
-	external: boolean;
+	external: boolean | 'absolute';
 	id: string;
 }
 
@@ -228,7 +228,7 @@ export interface ResolvedIdMap {
 }
 
 interface PartialResolvedId extends Partial<PartialNull<ModuleOptions>> {
-	external?: boolean;
+	external?: boolean | 'absolute' | 'relative';
 	id: string;
 }
 
@@ -528,6 +528,7 @@ export interface InputOptions {
 	/** @deprecated Use the "inlineDynamicImports" output option instead. */
 	inlineDynamicImports?: boolean;
 	input?: InputOption;
+	makeAbsoluteExternalsRelative?: boolean | 'ifRelativeSource';
 	/** @deprecated Use the "manualChunks" output option instead. */
 	manualChunks?: ManualChunksOption;
 	moduleContext?: ((id: string) => string | null | undefined) | { [id: string]: string };
@@ -554,6 +555,7 @@ export interface NormalizedInputOptions {
 	/** @deprecated Use the "inlineDynamicImports" output option instead. */
 	inlineDynamicImports: boolean | undefined;
 	input: string[] | { [entryAlias: string]: string };
+	makeAbsoluteExternalsRelative: boolean | 'ifRelativeSource';
 	/** @deprecated Use the "manualChunks" output option instead. */
 	manualChunks: ManualChunksOption | undefined;
 	moduleContext: (id: string) => string;

src/utils/options/mergeOptions.ts

@@ -106,6 +106,7 @@ function mergeInputOptions(
 		external: getExternal(config, overrides),
 		inlineDynamicImports: getOption('inlineDynamicImports'),
 		input: getOption('input') || [],
+		makeAbsoluteExternalsRelative: getOption('makeAbsoluteExternalsRelative'),
 		manualChunks: getOption('manualChunks'),
 		moduleContext: getOption('moduleContext'),
 		onwarn: getOnWarn(config, defaultOnWarnHandler),

src/utils/options/normalizeInputOptions.ts

@@ -49,6 +49,8 @@ export function normalizeInputOptions(
 		external: getIdMatcher(config.external as ExternalOption),
 		inlineDynamicImports: getInlineDynamicImports(config, onwarn, strictDeprecations),
 		input: getInput(config),
+		makeAbsoluteExternalsRelative:
+			(config.makeAbsoluteExternalsRelative as boolean | 'ifRelativeSource' | undefined) ?? true,
 		manualChunks: getManualChunks(config, onwarn, strictDeprecations),
 		moduleContext: getModuleContext(config, context),
 		onwarn,
@@ -262,7 +264,7 @@ const getTreeshake = (
 				warn
 			),
 			propertyReadSideEffects:
-				configTreeshake.propertyReadSideEffects === 'always' && 'always' ||
+				(configTreeshake.propertyReadSideEffects === 'always' && 'always') ||
 				configTreeshake.propertyReadSideEffects !== false,
 			tryCatchDeoptimization: configTreeshake.tryCatchDeoptimization !== false,
 			unknownGlobalSideEffects: configTreeshake.unknownGlobalSideEffects !== false

test/form/samples/make-absolute-externals-relative/make-relative-false/_config.js

@@ -0,0 +1,45 @@
+const path = require('path');
+const assert = require('assert');
+
+const ID_MAIN = path.join(__dirname, 'main.js');
+
+module.exports = {
+	description: 'does not normalize external paths when set to false',
+	options: {
+		makeAbsoluteExternalsRelative: false,
+		external(id) {
+			if (
+				[
+					'./relativeUnresolved.js',
+					'../relativeUnresolved.js',
+					'/absolute.js',
+					'/pluginAbsolute.js'
+				].includes(id)
+			)
+				return true;
+		},
+		plugins: {
+			async buildStart() {
+				const testExternal = async (source, expected) =>
+					assert.deepStrictEqual((await this.resolve(source, ID_MAIN)).external, expected, source);
+
+				await testExternal('./relativeUnresolved.js', true);
+				await testExternal('/absolute.js', 'absolute');
+				await testExternal('./pluginDirect.js', true);
+				await testExternal('/pluginDifferentAbsolute.js', 'absolute');
+				await testExternal('./pluginTrue.js', 'absolute');
+				await testExternal('./pluginForceAbsolute.js', 'absolute');
+				await testExternal('./pluginForceRelative.js', true);
+			},
+			resolveId(source) {
+				if (source.endsWith('/pluginDirect.js')) return false;
+				if (source.endsWith('/pluginDifferentAbsolute.js')) return '/pluginAbsolute.js';
+				if (source.endsWith('/pluginTrue.js')) return { id: '/pluginTrue.js', external: true };
+				if (source.endsWith('/pluginForceAbsolute.js'))
+					return { id: '/pluginForceAbsolute.js', external: 'absolute' };
+				if (source.endsWith('/pluginForceRelative.js'))
+					return { id: path.join(__dirname, 'pluginForceRelative.js'), external: 'relative' };
+			}
+		}
+	}
+};