refactor: use fs.promises in resolve id, Part 4 (#4386)

* refactor: use fs.promises in resolve id

* Make chunk variable names deterministic, sort watchfiles in tests

* Fix order of mixed export warnings

* Make tests Node 10 compatible

* remove test generated file

Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com>
Co-authored-by: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com>

cli/run/batchWarnings.ts

@@ -142,6 +142,7 @@ const deferredHandlers: {
 		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:'));
+		warnings.sort((a, b) => (a.id! < b.id! ? -1 : 1));
 		const displayedWarnings = warnings.length > 5 ? warnings.slice(0, 3) : warnings;
 		for (const warning of displayedWarnings) {
 			stderr(relativeId(warning.id!));

src/Chunk.ts

@@ -329,9 +329,14 @@ export default class Chunk {
 			}
 		}
 		for (const module of entryModules) {
-			const requiredFacades: FacadeName[] = Array.from(module.userChunkNames, name => ({
-				name
-			}));
+			const requiredFacades: FacadeName[] = Array.from(
+				new Set(
+					module.chunkNames.filter(({ isUserDefined }) => isUserDefined).map(({ name }) => name)
+				),
+				name => ({
+					name
+				})
+			);
 			if (requiredFacades.length === 0 && module.isUserDefinedEntryPoint) {
 				requiredFacades.push({});
 			}
@@ -959,7 +964,7 @@ export default class Chunk {
 			this.dynamicEntryModules[0] ||
 			this.orderedModules[this.orderedModules.length - 1];
 		if (moduleForNaming) {
-			return moduleForNaming.chunkName || getAliasName(moduleForNaming.id);
+			return getChunkNameFromModule(moduleForNaming);
 		}
 		return 'chunk';
 	}
@@ -1398,7 +1403,11 @@ export default class Chunk {
 }
 
 function getChunkNameFromModule(module: Module): string {
-	return module.chunkName || getAliasName(module.id);
+	return (
+		module.chunkNames.find(({ isUserDefined }) => isUserDefined)?.name ??
+		module.chunkNames[0]?.name ??
+		getAliasName(module.id)
+	);
 }
 
 const QUERY_HASH_REGEX = /[?#]/;

src/Module.ts

@@ -189,7 +189,11 @@ export default class Module {
 	readonly alternativeReexportModules = new Map<Variable, Module>();
 	ast: Program | null = null;
 	readonly chunkFileNames = new Set<string>();
-	chunkName: string | null = null;
+	chunkNames: {
+		isUserDefined: boolean;
+		name: string;
+		priority: number;
+	}[] = [];
 	readonly cycles = new Set<symbol>();
 	readonly dependencies = new Set<Module | ExternalModule>();
 	readonly dynamicDependencies = new Set<Module | ExternalModule>();
@@ -222,7 +226,6 @@ export default class Module {
 	declare sourcemapChain: DecodedSourceMapOrMissing[];
 	readonly sources = new Set<string>();
 	declare transformFiles?: EmittedFile[];
-	readonly userChunkNames = new Set<string>();
 	usesTopLevelAwait = false;
 
 	private allExportNames: Set<string> | null = null;

src/ModuleLoader.ts

@@ -70,6 +70,7 @@ export class ModuleLoader {
 	private latestLoadModulesPromise: Promise<unknown> = Promise.resolve();
 	private readonly moduleLoadPromises = new Map<Module, LoadModulePromise>();
 	private readonly modulesWithLoadedDependencies = new Set<Module>();
+	private nextChunkNamePriority = 0;
 	private nextEntryModuleIndex = 0;
 	private readonly readQueue: Queue<LoadResult>;
 
@@ -104,27 +105,38 @@ export class ModuleLoader {
 	}> {
 		const firstEntryModuleIndex = this.nextEntryModuleIndex;
 		this.nextEntryModuleIndex += unresolvedEntryModules.length;
+		const firstChunkNamePriority = this.nextChunkNamePriority;
+		this.nextChunkNamePriority += unresolvedEntryModules.length;
 		const newEntryModules = await this.extendLoadModulesPromise(
 			Promise.all(
 				unresolvedEntryModules.map(({ id, importer }) =>
 					this.loadEntryModule(id, true, importer, null)
 				)
 			).then(entryModules => {
-				let moduleIndex = firstEntryModuleIndex;
 				for (let index = 0; index < entryModules.length; index++) {
 					const entryModule = entryModules[index];
 					entryModule.isUserDefinedEntryPoint =
 						entryModule.isUserDefinedEntryPoint || isUserDefined;
-					addChunkNamesToModule(entryModule, unresolvedEntryModules[index], isUserDefined);
+					addChunkNamesToModule(
+						entryModule,
+						unresolvedEntryModules[index],
+						isUserDefined,
+						firstChunkNamePriority + index
+					);
 					const existingIndexedModule = this.indexedEntryModules.find(
 						indexedModule => indexedModule.module === entryModule
 					);
 					if (!existingIndexedModule) {
-						this.indexedEntryModules.push({ index: moduleIndex, module: entryModule });
+						this.indexedEntryModules.push({
+							index: firstEntryModuleIndex + index,
+							module: entryModule
+						});
 					} else {
-						existingIndexedModule.index = Math.min(existingIndexedModule.index, moduleIndex);
+						existingIndexedModule.index = Math.min(
+							existingIndexedModule.index,
+							firstEntryModuleIndex + index
+						);
 					}
-					moduleIndex++;
 				}
 				this.indexedEntryModules.sort(({ index: indexA }, { index: indexB }) =>
 					indexA > indexB ? 1 : -1
@@ -207,10 +219,11 @@ export class ModuleLoader {
 		unresolvedModule: UnresolvedModule,
 		implicitlyLoadedAfter: readonly string[]
 	): Promise<Module> {
+		const chunkNamePriority = this.nextChunkNamePriority++;
 		return this.extendLoadModulesPromise(
 			this.loadEntryModule(unresolvedModule.id, false, unresolvedModule.importer, null).then(
 				async entryModule => {
-					addChunkNamesToModule(entryModule, unresolvedModule, false);
+					addChunkNamesToModule(entryModule, unresolvedModule, false, chunkNamePriority);
 					if (!entryModule.info.isEntry) {
 						this.implicitEntryModules.add(entryModule);
 						const implicitlyLoadedAfterModules = await Promise.all(
@@ -688,17 +701,16 @@ function normalizeRelativeExternalId(source: string, importer: string | undefine
 function addChunkNamesToModule(
 	module: Module,
 	{ fileName, name }: UnresolvedModule,
-	isUserDefined: boolean
+	isUserDefined: boolean,
+	priority: number
 ): void {
 	if (fileName !== null) {
 		module.chunkFileNames.add(fileName);
 	} else if (name !== null) {
-		if (module.chunkName === null) {
-			module.chunkName = name;
-		}
-		if (isUserDefined) {
-			module.userChunkNames.add(name);
-		}
+		// Always keep chunkNames sorted by priority
+		let namePosition = 0;
+		while (module.chunkNames[namePosition]?.priority < priority) namePosition++;
+		module.chunkNames.splice(namePosition, 0, { isUserDefined, name, priority });
 	}
 }
 

src/utils/resolveId.ts

@@ -1,6 +1,6 @@
 import type { CustomPluginOptions, Plugin, ResolvedId, ResolveIdResult } from '../rollup/types';
 import type { PluginDriver } from './PluginDriver';
-import { lstatSync, readdirSync, realpathSync } from './fs';
+import { promises as fs } from './fs';
 import { basename, dirname, isAbsolute, resolve } from './path';
 import { resolveIdViaPlugins } from './resolveIdViaPlugins';
 
@@ -45,23 +45,26 @@ export async function resolveId(
 	);
 }
 
-function addJsExtensionIfNecessary(file: string, preserveSymlinks: boolean): string | undefined {
+async function addJsExtensionIfNecessary(
+	file: string,
+	preserveSymlinks: boolean
+): Promise<string | undefined> {
 	return (
-		findFile(file, preserveSymlinks) ??
-		findFile(file + '.mjs', preserveSymlinks) ??
-		findFile(file + '.js', preserveSymlinks)
+		(await findFile(file, preserveSymlinks)) ??
+		(await findFile(file + '.mjs', preserveSymlinks)) ??
+		(await findFile(file + '.js', preserveSymlinks))
 	);
 }
 
-function findFile(file: string, preserveSymlinks: boolean): string | undefined {
+async function findFile(file: string, preserveSymlinks: boolean): Promise<string | undefined> {
 	try {
-		const stats = lstatSync(file);
+		const stats = await fs.lstat(file);
 		if (!preserveSymlinks && stats.isSymbolicLink())
-			return findFile(realpathSync(file), preserveSymlinks);
+			return await findFile(await fs.realpath(file), preserveSymlinks);
 		if ((preserveSymlinks && stats.isSymbolicLink()) || stats.isFile()) {
 			// check case
 			const name = basename(file);
-			const files = readdirSync(dirname(file));
+			const files = await fs.readdir(dirname(file));
 
 			if (files.includes(name)) return file;
 		}

test/chunking-form/samples/emit-file/emit-chunk-name-conflict/_expected/amd/mainChunk.js

@@ -1,5 +1,5 @@
-define(['./generated-name', './generated-secondName', './generated-name2'], (function (name, secondName, name$1) { 'use strict';
+define(['./generated-name', './generated-firstName', './generated-name2'], (function (name, firstName, name$1) { 'use strict';
 
-	console.log('main', name, secondName, name$1);
+	console.log('main', name, firstName, name$1);
 
 }));

test/chunking-form/samples/emit-file/emit-chunk-name-conflict/_expected/amd/other.js

@@ -1,4 +1,4 @@
-define(['./generated-name', './generated-secondName', './generated-name2', './mainChunk'], (function (name, secondName, name$1, mainChunk) { 'use strict';
+define(['./generated-name', './generated-firstName', './generated-name2', './mainChunk'], (function (name, firstName, name$1, mainChunk) { 'use strict';
 
 
 

test/chunking-form/samples/emit-file/emit-chunk-name-conflict/_expected/cjs/mainChunk.js

@@ -1,7 +1,7 @@
 'use strict';
 
 var name = require('./generated-name.js');
-var secondName = require('./generated-secondName.js');
+var firstName = require('./generated-firstName.js');
 var name$1 = require('./generated-name2.js');
 
-console.log('main', name, secondName, name$1);
+console.log('main', name, firstName, name$1);

test/chunking-form/samples/emit-file/emit-chunk-name-conflict/_expected/cjs/other.js

@@ -1,7 +1,7 @@
 'use strict';
 
 require('./generated-name.js');
-require('./generated-secondName.js');
+require('./generated-firstName.js');
 require('./generated-name2.js');
 require('./mainChunk.js');
 

test/chunking-form/samples/emit-file/emit-chunk-name-conflict/_expected/es/mainChunk.js

@@ -1,5 +1,5 @@
 import value1 from './generated-name.js';
-import value2 from './generated-secondName.js';
+import value2 from './generated-firstName.js';
 import value3 from './generated-name2.js';
 
 console.log('main', value1, value2, value3);

test/chunking-form/samples/emit-file/emit-chunk-name-conflict/_expected/es/other.js

@@ -1,4 +1,4 @@
 import './generated-name.js';
-import './generated-secondName.js';
+import './generated-firstName.js';
 import './generated-name2.js';
 import './mainChunk.js';

test/chunking-form/samples/emit-file/emit-chunk-name-conflict/_expected/system/mainChunk.js

@@ -1,4 +1,4 @@
-System.register(['./generated-name.js', './generated-secondName.js', './generated-name2.js'], (function () {
+System.register(['./generated-name.js', './generated-firstName.js', './generated-name2.js'], (function () {
 	'use strict';
 	var value1, value2, value3;
 	return {

test/chunking-form/samples/emit-file/emit-chunk-name-conflict/_expected/system/other.js

@@ -1,4 +1,4 @@
-System.register(['./generated-name.js', './generated-secondName.js', './generated-name2.js', './mainChunk.js'], (function () {
+System.register(['./generated-name.js', './generated-firstName.js', './generated-name2.js', './mainChunk.js'], (function () {
 	'use strict';
 	return {
 		setters: [function () {}, function () {}, function () {}, function () {}],

test/cli/samples/warn-mixed-exports-multiple/_config.js

@@ -9,9 +9,9 @@ module.exports = {
 			'(!) Mixing named and default exports\n' +
 				'https://rollupjs.org/guide/en/#outputexports\n' +
 				'The following entry modules are using named and default exports together:\n' +
-				'main.js\n' +
 				'lib1.js\n' +
 				'lib2.js\n' +
+				'lib3.js\n' +
 				'...and 3 other entry modules\n' +
 				'\n' +
 				"Consumers of your bundle will have to use chunk['default'] to access their default export, which may not be what you want. Use `output.exports: 'named'` to disable this warning\n"

test/cli/samples/watch/watch-config-error/_config.js

@@ -22,7 +22,8 @@ module.exports = {
 		);
 	},
 	after() {
-		unlinkSync(configFile);
+		// synchronous sometimes does not seem to work, probably because the watch is not yet removed properly
+		setTimeout(() => unlinkSync(configFile), 300);
 	},
 	abortOnStderr(data) {
 		if (data.includes(`created _actual${path.sep}main1.js`)) {

test/function/samples/circular-missed-reexports-2/_config.js

@@ -12,9 +12,9 @@ module.exports = {
 		message:
 			'"doesNotExist" cannot be exported from dep1.js as it is a reexport that references itself.',
 		watchFiles: [
-			path.join(__dirname, 'main.js'),
 			path.join(__dirname, 'dep1.js'),
-			path.join(__dirname, 'dep2.js')
+			path.join(__dirname, 'dep2.js'),
+			path.join(__dirname, 'main.js')
 		]
 	}
 };

test/function/samples/conflicting-reexports/named-import/_config.js

@@ -21,9 +21,9 @@ module.exports = {
 		pos: 9,
 		url: 'https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module',
 		watchFiles: [
+			path.join(__dirname, 'first.js'),
 			ID_MAIN,
 			path.join(__dirname, 'reexport.js'),
-			path.join(__dirname, 'first.js'),
 			path.join(__dirname, 'second.js')
 		]
 	}

test/function/samples/default-not-reexported/_config.js

@@ -8,9 +8,9 @@ module.exports = {
 		id: path.join(__dirname, 'main.js'),
 		pos: 7,
 		watchFiles: [
-			path.join(__dirname, 'main.js'),
+			path.join(__dirname, 'bar.js'),
 			path.join(__dirname, 'foo.js'),
-			path.join(__dirname, 'bar.js')
+			path.join(__dirname, 'main.js')
 		],
 		loc: {
 			file: path.join(__dirname, 'main.js'),

test/function/samples/deprecated/emit-chunk/chunk-filename-not-available/_config.js

@@ -1,5 +1,3 @@
-const path = require('path');
-
 module.exports = {
 	description: 'Throws when accessing the filename before it has been generated',
 	options: {
@@ -19,7 +17,6 @@ module.exports = {
 		message:
 			'Plugin error - Unable to get file name for chunk "chunk.js". Ensure that generate is called first.',
 		plugin: 'test-plugin',
-		pluginCode: 'CHUNK_NOT_GENERATED',
-		watchFiles: [path.join(__dirname, 'chunk.js')]
+		pluginCode: 'CHUNK_NOT_GENERATED'
 	}
 };