refactor: some code and type fixes (#4413)

* use readonly set

* refactor: re-use Array.prototype.map, remove Array.from map function

* refactor: remove else block

* refactor: use logical nullish assignment

* fix: types

* fix: add generic parameters for Map

* refactor: simplify

* fix: remove type assertion

* refactor: create map instance only once

* refactor: use object.entries over .keys

* refactor: simplify

* refactor: simplify

* remove unnecassary .values call

* refactor: simplify, use substring

* remove unnecassary .entries

* refactor: simplify

* fix: replace deprecated .substr with .substring

* Revert "refactor: re-use Array.prototype.map, remove Array.from map function"

This reverts commit 055f4a0.

* add comment

* fix hash update

* remove initial id base assignment

* remove breaks, return early

* remove default case

* return early

* add generic type parameters, remove eslint-disable

* use object shorthand

* order nit

* reference from this

* simplify, use map/filter

Author: dnalborczyk

cli/run/batchWarnings.ts

@@ -14,11 +14,11 @@ export interface BatchWarnings {
 
 export default function batchWarnings(): BatchWarnings {
 	let count = 0;
-	let deferredWarnings = new Map<keyof typeof deferredHandlers, RollupWarning[]>();
+	const deferredWarnings = new Map<keyof typeof deferredHandlers, RollupWarning[]>();
 	let warningOccurred = false;
 
 	return {
-		add: (warning: RollupWarning) => {
+		add(warning: RollupWarning) {
 			count += 1;
 			warningOccurred = true;
 
@@ -48,7 +48,7 @@ export default function batchWarnings(): BatchWarnings {
 			return count;
 		},
 
-		flush: () => {
+		flush() {
 			if (count === 0) return;
 
 			const codes = Array.from(deferredWarnings.keys()).sort(
@@ -59,7 +59,7 @@ export default function batchWarnings(): BatchWarnings {
 				deferredHandlers[code](deferredWarnings.get(code)!);
 			}
 
-			deferredWarnings = new Map();
+			deferredWarnings.clear();
 			count = 0;
 		},
 
@@ -72,7 +72,7 @@ export default function batchWarnings(): BatchWarnings {
 const immediateHandlers: {
 	[code: string]: (warning: RollupWarning) => void;
 } = {
-	MISSING_NODE_BUILTINS: warning => {
+	MISSING_NODE_BUILTINS(warning) {
 		title(`Missing shims for Node.js built-ins`);
 
 		stderr(
@@ -82,7 +82,7 @@ const immediateHandlers: {
 		);
 	},
 
-	UNKNOWN_OPTION: warning => {
+	UNKNOWN_OPTION(warning) {
 		title(`You have passed an unrecognized option`);
 		stderr(warning.message);
 	}
@@ -138,7 +138,7 @@ const deferredHandlers: {
 		}
 	},
 
-	MIXED_EXPORTS: warnings => {
+	MIXED_EXPORTS(warnings) {
 		title('Mixing named and default exports');
 		info(`https://rollupjs.org/guide/en/#outputexports`);
 		stderr(bold('The following entry modules are using named and default exports together:'));
@@ -204,9 +204,7 @@ const deferredHandlers: {
 		title(`Broken sourcemap`);
 		info('https://rollupjs.org/guide/en/#warning-sourcemap-is-likely-to-be-incorrect');
 
-		const plugins = [
-			...new Set(warnings.map(warning => warning.plugin).filter(Boolean))
-		] as string[];
+		const plugins = [...new Set(warnings.map(({ plugin }) => plugin).filter(Boolean))] as string[];
 		stderr(
 			`Plugins that transform code (such as ${printQuotedStringList(
 				plugins
@@ -224,13 +222,12 @@ const deferredHandlers: {
 		title('Unresolved dependencies');
 		info('https://rollupjs.org/guide/en/#warning-treating-module-as-external-dependency');
 
-		const dependencies = new Map();
+		const dependencies = new Map<string, string[]>();
 		for (const warning of warnings) {
-			getOrCreate(dependencies, warning.source, () => []).push(warning.importer);
+			getOrCreate(dependencies, warning.source, () => []).push(warning.importer!);
 		}
 
-		for (const dependency of dependencies.keys()) {
-			const importers = dependencies.get(dependency);
+		for (const [dependency, importers] of dependencies) {
 			stderr(`${bold(dependency)} (imported by ${importers.join(', ')})`);
 		}
 	},
@@ -243,7 +240,7 @@ const deferredHandlers: {
 					' imported from external module "' +
 					warning.source +
 					'" but never used in ' +
-					printQuotedStringList((warning.sources as string[]).map(id => relativeId(id)))
+					printQuotedStringList(warning.sources!.map(id => relativeId(id)))
 			);
 		}
 	}

cli/run/index.ts

@@ -1,3 +1,4 @@
+import { env } from 'process';
 import type { MergedRollupOptions } from '../../src/rollup/types';
 import { isWatchEnabled } from '../../src/utils/options/mergeOptions';
 import { getAliasName } from '../../src/utils/relativeId';
@@ -48,11 +49,7 @@ export default async function runRollup(command: Record<string, any>): Promise<v
 		environment.forEach((arg: string) => {
 			arg.split(',').forEach((pair: string) => {
 				const [key, ...value] = pair.split(':');
-				if (value.length) {
-					process.env[key] = value.join(':');
-				} else {
-					process.env[key] = String(true);
-				}
+				env[key] = value.length === 0 ? String(true) : value.join(':');
 			});
 		});
 	}

cli/run/timings.ts

@@ -3,10 +3,9 @@ import type { SerializedTimings } from '../../src/rollup/types';
 import { bold, underline } from '../../src/utils/colors';
 
 export function printTimings(timings: SerializedTimings): void {
-	Object.keys(timings).forEach(label => {
+	Object.entries(timings).forEach(([label, [time, memory, total]]) => {
 		const appliedColor =
 			label[0] === '#' ? (label[1] !== '#' ? underline : bold) : (text: string) => text;
-		const [time, memory, total] = timings[label];
 		const row = `${label}: ${time.toFixed(0)}ms, ${prettyBytes(memory)} / ${prettyBytes(total)}`;
 		console.info(appliedColor(row));
 	});

src/Bundle.ts

@@ -35,7 +35,7 @@ export default class Bundle {
 
 	constructor(
 		private readonly outputOptions: NormalizedOutputOptions,
-		private readonly unsetOptions: Set<string>,
+		private readonly unsetOptions: ReadonlySet<string>,
 		private readonly inputOptions: NormalizedInputOptions,
 		private readonly pluginDriver: PluginDriver,
 		private readonly graph: Graph
@@ -189,14 +189,14 @@ export default class Bundle {
 				);
 				(file as OutputAsset).type = 'asset';
 			}
-			if (this.outputOptions.validate && typeof (file as OutputChunk).code == 'string') {
+			if (this.outputOptions.validate && 'code' in file) {
 				try {
-					this.graph.contextParse((file as OutputChunk).code, {
+					this.graph.contextParse(file.code, {
 						allowHashBang: true,
 						ecmaVersion: 'latest'
 					});
 				} catch (err: any) {
-					this.inputOptions.onwarn(errChunkInvalid(file as OutputChunk, err));
+					this.inputOptions.onwarn(errChunkInvalid(file, err));
 				}
 			}
 		}

src/Chunk.ts

@@ -319,7 +319,7 @@ export default class Chunk {
 		const facades: Chunk[] = [];
 		const entryModules = new Set([...this.entryModules, ...this.implicitEntryModules]);
 		const exposedVariables = new Set<Variable>(
-			this.dynamicEntryModules.map(module => module.namespace)
+			this.dynamicEntryModules.map(({ namespace }) => namespace)
 		);
 		for (const module of entryModules) {
 			if (module.preserveSignature) {
@@ -333,6 +333,7 @@ export default class Chunk {
 				new Set(
 					module.chunkNames.filter(({ isUserDefined }) => isUserDefined).map(({ name }) => name)
 				),
+				// mapping must run after Set 'name' dedupe
 				name => ({
 					name
 				})
@@ -451,7 +452,7 @@ export default class Chunk {
 			const extension = extname(sanitizedId);
 			const fileName = renderNamePattern(pattern, 'output.entryFileNames', {
 				assetExtname: () => (NON_ASSET_EXTENSIONS.includes(extension) ? '' : extension),
-				ext: () => extension.substr(1),
+				ext: () => extension.substring(1),
 				extname: () => extension,
 				format: () => options.format as string,
 				name: () => this.getChunkName()
@@ -467,7 +468,7 @@ export default class Chunk {
 			const extension = extname(sanitizedId);
 			const fileName = renderNamePattern(pattern, 'output.entryFileNames', {
 				assetExtname: () => (NON_ASSET_EXTENSIONS.includes(extension) ? '' : extension),
-				ext: () => extension.substr(1),
+				ext: () => extension.substring(1),
 				extname: () => extension,
 				format: () => options.format as string,
 				name: () => getAliasName(sanitizedId)
@@ -1162,7 +1163,7 @@ export default class Chunk {
 			let imported: string;
 			let needsLiveBinding = false;
 			if (exportName[0] === '*') {
-				const id = exportName.substr(1);
+				const id = exportName.substring(1);
 				if (interop(id) === 'defaultOnly') {
 					this.inputOptions.onwarn(errUnexpectedNamespaceReexport(id));
 				}

src/Module.ts

@@ -264,12 +264,13 @@ export default class Module {
 		const {
 			dynamicImports,
 			dynamicImporters,
-			reexportDescriptions,
 			implicitlyLoadedAfter,
 			implicitlyLoadedBefore,
-			sources,
-			importers
+			importers,
+			reexportDescriptions,
+			sources
 		} = this;
+
 		this.info = {
 			ast: null,
 			code: null,
@@ -281,13 +282,7 @@ export default class Module {
 			get dynamicallyImportedIds() {
 				// We cannot use this.dynamicDependencies because this is needed before
 				// dynamicDependencies are populated
-				const dynamicallyImportedIds: string[] = [];
-				for (const { id } of dynamicImports) {
-					if (id) {
-						dynamicallyImportedIds.push(id);
-					}
-				}
-				return dynamicallyImportedIds;
+				return dynamicImports.map(({ id }) => id).filter((id): id is string => id != null);
 			},
 			get dynamicImporters() {
 				return dynamicImporters.sort();
@@ -305,7 +300,7 @@ export default class Module {
 					false,
 					options
 				);
-				return module.info.moduleSideEffects;
+				return this.moduleSideEffects;
 			},
 			id,
 			get implicitlyLoadedAfterOneOf() {

src/ModuleLoader.ts

@@ -439,9 +439,8 @@ export class ModuleLoader {
 				return error(errInternalIdCannotBeExternal(source, importer));
 			}
 			return Promise.resolve(externalModule);
-		} else {
-			return this.fetchModule(resolvedId, importer, false, false);
 		}
+		return this.fetchModule(resolvedId, importer, false, false);
 	}
 
 	private async fetchStaticDependencies(
@@ -673,13 +672,11 @@ export class ModuleLoader {
 			} as ResolvedId;
 		}
 		if (resolution == null) {
-			return (module.resolvedIds[specifier] =
-				module.resolvedIds[specifier] ||
-				this.handleResolveId(
-					await this.resolveId(specifier, module.id, EMPTY_OBJECT, false),
-					specifier,
-					module.id
-				));
+			return (module.resolvedIds[specifier] ??= this.handleResolveId(
+				await this.resolveId(specifier, module.id, EMPTY_OBJECT, false),
+				specifier,
+				module.id
+			));
 		}
 		return this.handleResolveId(
 			this.getResolvedIdWithDefaults(

src/ast/nodes/MetaProperty.ts

@@ -43,7 +43,7 @@ export default class MetaProperty extends NodeBase {
 	getReferencedFileName(outputPluginDriver: PluginDriver): string | null {
 		const metaProperty = this.metaProperty as string | null;
 		if (metaProperty && metaProperty.startsWith(FILE_PREFIX)) {
-			return outputPluginDriver.getFileName(metaProperty.substr(FILE_PREFIX.length));
+			return outputPluginDriver.getFileName(metaProperty.substring(FILE_PREFIX.length));
 		}
 		return null;
 	}
@@ -91,23 +91,23 @@ export default class MetaProperty extends NodeBase {
 			let chunkReferenceId: string | null = null;
 			let fileName: string;
 			if (metaProperty.startsWith(FILE_PREFIX)) {
-				referenceId = metaProperty.substr(FILE_PREFIX.length);
+				referenceId = metaProperty.substring(FILE_PREFIX.length);
 				fileName = outputPluginDriver.getFileName(referenceId);
 			} else if (metaProperty.startsWith(ASSET_PREFIX)) {
 				warnDeprecation(
 					`Using the "${ASSET_PREFIX}" prefix to reference files is deprecated. Use the "${FILE_PREFIX}" prefix instead.`,
 					true,
 					this.context.options
 				);
-				assetReferenceId = metaProperty.substr(ASSET_PREFIX.length);
+				assetReferenceId = metaProperty.substring(ASSET_PREFIX.length);
 				fileName = outputPluginDriver.getFileName(assetReferenceId);
 			} else {
 				warnDeprecation(
 					`Using the "${CHUNK_PREFIX}" prefix to reference files is deprecated. Use the "${FILE_PREFIX}" prefix instead.`,
 					true,
 					this.context.options
 				);
-				chunkReferenceId = metaProperty.substr(CHUNK_PREFIX.length);
+				chunkReferenceId = metaProperty.substring(CHUNK_PREFIX.length);
 				fileName = outputPluginDriver.getFileName(chunkReferenceId);
 			}
 			const relativePath = normalize(relative(dirname(chunkId), fileName));

src/ast/values.ts

@@ -236,9 +236,8 @@ export function getLiteralMembersForValue<T extends LiteralValue = LiteralValue>
 			return literalNumberMembers;
 		case 'string':
 			return literalStringMembers;
-		default:
-			return Object.create(null);
 	}
+	return Object.create(null);
 }
 
 export function hasMemberEffectWhenCalled(

src/rollup/rollup.ts

@@ -137,12 +137,11 @@ function applyOptionHook(watchMode: boolean) {
 }
 
 function normalizePlugins(plugins: readonly Plugin[], anonymousPrefix: string): void {
-	for (let pluginIndex = 0; pluginIndex < plugins.length; pluginIndex++) {
-		const plugin = plugins[pluginIndex];
+	plugins.forEach((plugin, index) => {
 		if (!plugin.name) {
-			plugin.name = `${anonymousPrefix}${pluginIndex + 1}`;
+			plugin.name = `${anonymousPrefix}${index + 1}`;
 		}
-	}
+	});
 }
 
 async function handleGenerateWrite(