Prefer locally defined exports and reexports over external namespaces

cli/run/batchWarnings.ts

@@ -1,6 +1,7 @@
 import { bold, gray, yellow } from 'colorette';
 import { RollupWarning } from '../../src/rollup/types';
 import { getOrCreate } from '../../src/utils/getOrCreate';
+import { printQuotedStringList } from '../../src/utils/printStringList';
 import relativeId from '../../src/utils/relativeId';
 import { stderr } from '../logging';
 
@@ -79,15 +80,10 @@ const immediateHandlers: {
 	MISSING_NODE_BUILTINS: warning => {
 		title(`Missing shims for Node.js built-ins`);
 
-		const detail =
-			warning.modules!.length === 1
-				? `'${warning.modules![0]}'`
-				: `${warning
-						.modules!.slice(0, -1)
-						.map((name: string) => `'${name}'`)
-						.join(', ')} and '${warning.modules!.slice(-1)}'`;
 		stderr(
-			`Creating a browser bundle that depends on ${detail}. You might need to include https://github.com/ionic-team/rollup-plugin-node-polyfills`
+			`Creating a browser bundle that depends on ${printQuotedStringList(
+				warning.modules!
+			)}. You might need to include https://github.com/ionic-team/rollup-plugin-node-polyfills`
 		);
 	}
 };
@@ -162,11 +158,11 @@ const deferredHandlers: {
 		title(`Conflicting re-exports`);
 		for (const warning of warnings) {
 			stderr(
-				`${bold(relativeId(warning.reexporter!))} re-exports '${
+				`"${bold(relativeId(warning.reexporter!))}" re-exports "${
 					warning.name
-				}' from both ${relativeId(warning.sources![0])} and ${relativeId(
+				}" from both "${relativeId(warning.sources![0])}" and "${relativeId(
 					warning.sources![1]
-				)} (will be ignored)`
+				)}" (will be ignored)`
 			);
 		}
 	},
@@ -207,16 +203,14 @@ const deferredHandlers: {
 		title(`Broken sourcemap`);
 		info('https://rollupjs.org/guide/en/#warning-sourcemap-is-likely-to-be-incorrect');
 
-		const plugins = Array.from(new Set(warnings.map(w => w.plugin).filter(Boolean)));
-		const detail =
-			plugins.length > 1
-				? ` (such as ${plugins
-						.slice(0, -1)
-						.map(p => `'${p}'`)
-						.join(', ')} and '${plugins.slice(-1)}')`
-				: ` (such as '${plugins[0]}')`;
-
-		stderr(`Plugins that transform code${detail} should generate accompanying sourcemaps`);
+		const plugins = [
+			...new Set(warnings.map(warning => warning.plugin).filter(Boolean))
+		] as string[];
+		stderr(
+			`Plugins that transform code (such as ${printQuotedStringList(
+				plugins
+			)}) should generate accompanying sourcemaps`
+		);
 	},
 
 	THIS_IS_UNDEFINED(warnings) {

src/ExternalModule.ts

@@ -8,6 +8,8 @@ import {
 import { EMPTY_ARRAY } from './utils/blank';
 import { makeLegal } from './utils/identifierHelpers';
 import { normalize, relative } from './utils/path';
+import { printQuotedStringList } from './utils/printStringList';
+import relativeId from './utils/relativeId';
 
 export default class ExternalModule {
 	chunk: void;
@@ -99,32 +101,23 @@ export default class ExternalModule {
 			const declaration = this.declarations[name];
 			return !declaration.included && !this.reexported && !declaration.referenced;
 		});
-
 		if (unused.length === 0) return;
 
-		const names =
-			unused.length === 1
-				? `'${unused[0]}' is`
-				: `${unused
-						.slice(0, -1)
-						.map(name => `'${name}'`)
-						.join(', ')} and '${unused.slice(-1)}' are`;
-
 		const importersSet = new Set<string>();
 		for (const name of unused) {
-			const {importers, dynamicImporters} = this.declarations[name].module;
-
-			if (Array.isArray(importers)) importers.forEach(v => importersSet.add(v));
-			if (Array.isArray(dynamicImporters)) dynamicImporters.forEach(v => importersSet.add(v));
+			const { importers } = this.declarations[name].module;
+			for (const importer of importers) {
+				importersSet.add(importer);
+			}
 		}
-
-		const importersArray = Array.from(importersSet);
-
-		const importerList = ' in' + importersArray.map(s => `\n\t${s};`);
-
+		const importersArray = [...importersSet];
 		this.options.onwarn({
 			code: 'UNUSED_EXTERNAL_IMPORT',
-			message: `${names} imported from external module '${this.id}' but never used${importerList}`,
+			message: `${printQuotedStringList(unused, ['is', 'are'])} imported from external module "${
+				this.id
+			}" but never used in ${printQuotedStringList(
+				importersArray.map(importer => relativeId(importer))
+			)}.`,
 			names: unused,
 			source: this.id,
 			sources: importersArray

src/Module.ts

@@ -45,8 +45,10 @@ import {
 	RollupWarning,
 	TransformModuleJSON
 } from './rollup/types';
+import { EMPTY_OBJECT } from './utils/blank';
 import {
 	augmentCodeLocation,
+	errAmbiguousExternalNamespaces,
 	errCircularReexport,
 	errMissingExport,
 	errNamespaceConflict,
@@ -129,8 +131,11 @@ function findSourceMappingURLComments(ast: acorn.Node, code: string): [number, n
 
 		let sourcemappingUrlMatch;
 		const interStatmentCode = code.slice(start, end);
-		while (sourcemappingUrlMatch = SOURCEMAPPING_URL_COMMENT_RE.exec(interStatmentCode)) {
-			ret.push([start + sourcemappingUrlMatch.index, start + SOURCEMAPPING_URL_COMMENT_RE.lastIndex]);
+		while ((sourcemappingUrlMatch = SOURCEMAPPING_URL_COMMENT_RE.exec(interStatmentCode))) {
+			ret.push([
+				start + sourcemappingUrlMatch.index,
+				start + SOURCEMAPPING_URL_COMMENT_RE.lastIndex
+			]);
 		}
 	};
 
@@ -149,7 +154,8 @@ function getVariableForExportNameRecursive(
 	name: string,
 	importerForSideEffects: Module | undefined,
 	isExportAllSearch: boolean,
-	searchedNamesAndModules = new Map<string, Set<Module | ExternalModule>>()
+	searchedNamesAndModules = new Map<string, Set<Module | ExternalModule>>(),
+	skipExternalNamespaceReexports: boolean | undefined
 ): Variable | null {
 	const searchedModules = searchedNamesAndModules.get(name);
 	if (searchedModules) {
@@ -160,12 +166,12 @@ function getVariableForExportNameRecursive(
 	} else {
 		searchedNamesAndModules.set(name, new Set([target]));
 	}
-	return target.getVariableForExportName(
-		name,
+	return target.getVariableForExportName(name, {
 		importerForSideEffects,
 		isExportAllSearch,
-		searchedNamesAndModules
-	);
+		searchedNamesAndModules,
+		skipExternalNamespaceReexports
+	});
 }
 
 function getAndExtendSideEffectModules(variable: Variable, module: Module): Set<Module> {
@@ -253,6 +259,7 @@ export default class Module {
 	private exportNamesByVariable: Map<Variable, string[]> | null = null;
 	private exportShimVariable: ExportShimVariable = new ExportShimVariable(this);
 	private magicString!: MagicString;
+	private namespaceReexportsByName: Record<string, Variable | null> = Object.create(null);
 	private relevantDependencies: Set<Module | ExternalModule> | null = null;
 	private syntheticExports = new Map<string, SyntheticNamedExportVariable>();
 	private syntheticNamespace: Variable | null | undefined = null;
@@ -364,7 +371,10 @@ export default class Module {
 		) {
 			dependencyVariables = new Set(dependencyVariables);
 			for (const exportName of [...this.getReexports(), ...this.getExports()]) {
-				dependencyVariables.add(this.getVariableForExportName(exportName)!);
+				const exportedVariable = this.getVariableForExportName(exportName);
+				if (exportedVariable) {
+					dependencyVariables.add(exportedVariable);
+				}
 			}
 		}
 		for (let variable of dependencyVariables) {
@@ -482,9 +492,17 @@ export default class Module {
 
 	getVariableForExportName(
 		name: string,
-		importerForSideEffects?: Module,
-		isExportAllSearch?: boolean,
-		searchedNamesAndModules?: Map<string, Set<Module | ExternalModule>>
+		{
+			importerForSideEffects,
+			isExportAllSearch,
+			searchedNamesAndModules,
+			skipExternalNamespaceReexports
+		}: {
+			importerForSideEffects?: Module;
+			isExportAllSearch?: boolean;
+			searchedNamesAndModules?: Map<string, Set<Module | ExternalModule>>;
+			skipExternalNamespaceReexports?: boolean;
+		} = EMPTY_OBJECT
 	): Variable | null {
 		if (name[0] === '*') {
 			if (name.length === 1) {
@@ -505,7 +523,8 @@ export default class Module {
 				reexportDeclaration.localName,
 				importerForSideEffects,
 				false,
-				searchedNamesAndModules
+				searchedNamesAndModules,
+				false
 			);
 
 			if (!variable) {
@@ -539,27 +558,17 @@ export default class Module {
 		}
 
 		if (name !== 'default') {
-			let foundSyntheticDeclaration: SyntheticNamedExportVariable | null = null;
-			for (const module of this.exportAllModules) {
-				const declaration = getVariableForExportNameRecursive(
-					module,
-					name,
-					importerForSideEffects,
-					true,
-					searchedNamesAndModules
-				);
-
-				if (declaration) {
-					if (!(declaration instanceof SyntheticNamedExportVariable)) {
-						return declaration;
-					}
-					if (!foundSyntheticDeclaration) {
-						foundSyntheticDeclaration = declaration;
-					}
-				}
-			}
-			if (foundSyntheticDeclaration) {
-				return foundSyntheticDeclaration;
+			const foundNamespaceReexport =
+				name in this.namespaceReexportsByName
+					? this.namespaceReexportsByName[name]
+					: (this.namespaceReexportsByName[name] = this.getVariableFromNamespaceReexports(
+							name,
+							importerForSideEffects,
+							searchedNamesAndModules,
+							skipExternalNamespaceReexports
+					  ));
+			if (foundNamespaceReexport) {
+				return foundNamespaceReexport;
 			}
 		}
 
@@ -618,13 +627,15 @@ export default class Module {
 		}
 
 		for (const name of this.getReexports()) {
-			const variable = this.getVariableForExportName(name)!;
-			variable.deoptimizePath(UNKNOWN_PATH);
-			if (!variable.included) {
-				this.includeVariable(variable);
-			}
-			if (variable instanceof ExternalVariable) {
-				variable.module.reexported = true;
+			const variable = this.getVariableForExportName(name);
+			if (variable) {
+				variable.deoptimizePath(UNKNOWN_PATH);
+				if (!variable.included) {
+					this.includeVariable(variable);
+				}
+				if (variable instanceof ExternalVariable) {
+					variable.module.reexported = true;
+				}
 			}
 		}
 
@@ -798,10 +809,9 @@ export default class Module {
 				return otherModule.namespace;
 			}
 
-			const declaration = otherModule.getVariableForExportName(
-				importDeclaration.name,
-				importerForSideEffects || this
-			);
+			const declaration = otherModule.getVariableForExportName(importDeclaration.name, {
+				importerForSideEffects: importerForSideEffects || this
+			});
 
 			if (!declaration) {
 				return this.error(
@@ -837,7 +847,6 @@ export default class Module {
 		}
 	}
 
-
 	updateOptions({
 		meta,
 		moduleSideEffects,
@@ -1032,6 +1041,62 @@ export default class Module {
 		addSideEffectDependencies(alwaysCheckedDependencies);
 	}
 
+	private getVariableFromNamespaceReexports(
+		name: string,
+		importerForSideEffects?: Module,
+		searchedNamesAndModules?: Map<string, Set<Module | ExternalModule>>,
+		skipExternalNamespaceReexports = false
+	): Variable | null {
+		let foundSyntheticDeclaration: SyntheticNamedExportVariable | null = null;
+		const skipExternalNamespaceValues = new Set([true, skipExternalNamespaceReexports]);
+		for (const skipExternalNamespaces of skipExternalNamespaceValues) {
+			const foundDeclarations = new Set<Variable>();
+			for (const module of this.exportAllModules) {
+				if (module instanceof Module || !skipExternalNamespaces) {
+					const declaration = getVariableForExportNameRecursive(
+						module,
+						name,
+						importerForSideEffects,
+						true,
+						searchedNamesAndModules,
+						skipExternalNamespaces
+					);
+
+					if (declaration) {
+						if (!(declaration instanceof SyntheticNamedExportVariable)) {
+							foundDeclarations.add(declaration);
+						} else if (!foundSyntheticDeclaration) {
+							foundSyntheticDeclaration = declaration;
+						}
+					}
+				}
+			}
+			if (foundDeclarations.size === 1) {
+				return [...foundDeclarations][0];
+			}
+			if (foundDeclarations.size > 1) {
+				if (skipExternalNamespaces) {
+					return null;
+				}
+				const foundDeclarationList = [...(foundDeclarations as Set<ExternalVariable>)];
+				const usedDeclaration = foundDeclarationList[0];
+				this.options.onwarn(
+					errAmbiguousExternalNamespaces(
+						name,
+						this.id,
+						usedDeclaration.module.id,
+						foundDeclarationList.map(declaration => declaration.module.id)
+					)
+				);
+				return usedDeclaration;
+			}
+		}
+		if (foundSyntheticDeclaration) {
+			return foundSyntheticDeclaration;
+		}
+		return null;
+	}
+
 	private includeAndGetAdditionalMergedNamespaces(): Variable[] {
 		const mergedNamespaces: Variable[] = [];
 		for (const module of this.exportAllModules) {

src/ast/nodes/MemberExpression.ts

@@ -123,14 +123,9 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
 		if (path.length === 0) this.disallowNamespaceReassignment();
 		if (this.variable) {
 			this.variable.deoptimizePath(path);
-		} else {
-			const propertyKey = this.getPropertyKey();
-			if (propertyKey === UnknownKey) {
-				this.object.deoptimizePath(UNKNOWN_PATH);
-			} else {
-				this.wasPathDeoptimizedWhileOptimized = true;
-				this.object.deoptimizePath([propertyKey, ...path]);
-			}
+		} else if (!this.replacement) {
+			this.wasPathDeoptimizedWhileOptimized = true;
+			this.object.deoptimizePath([this.getPropertyKey(), ...path]);
 		}
 	}