Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from Feb 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions cli/run/batchWarnings.ts
Expand Up @@ -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!));
Expand Down
19 changes: 14 additions & 5 deletions src/Chunk.ts
Expand Up @@ -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({});
}
Expand Down Expand Up @@ -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';
}
Expand Down Expand Up @@ -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 = /[?#]/;
7 changes: 5 additions & 2 deletions src/Module.ts
Expand Up @@ -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>();
Expand Down Expand Up @@ -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;
Expand Down
38 changes: 25 additions & 13 deletions src/ModuleLoader.ts
Expand Up @@ -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>;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 });
}
}

Expand Down
21 changes: 12 additions & 9 deletions 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';

Expand Down Expand Up @@ -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;
}
Expand Down
@@ -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);

}));
@@ -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';



Expand Down
@@ -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);
@@ -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');

@@ -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);
@@ -1,4 +1,4 @@
import './generated-name.js';
import './generated-secondName.js';
import './generated-firstName.js';
import './generated-name2.js';
import './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 {
Expand Down
@@ -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 () {}],
Expand Down
2 changes: 1 addition & 1 deletion test/cli/samples/warn-mixed-exports-multiple/_config.js
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion test/cli/samples/watch/bundle-error/main.js

This file was deleted.

3 changes: 2 additions & 1 deletion test/cli/samples/watch/watch-config-error/_config.js
Expand Up @@ -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`)) {
Expand Down
4 changes: 2 additions & 2 deletions test/function/samples/circular-missed-reexports-2/_config.js
Expand Up @@ -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')
]
}
};
Expand Up @@ -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')
]
}
Expand Down
4 changes: 2 additions & 2 deletions test/function/samples/default-not-reexported/_config.js
Expand Up @@ -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'),
Expand Down
@@ -1,5 +1,3 @@
const path = require('path');

module.exports = {
description: 'Throws when accessing the filename before it has been generated',
options: {
Expand All @@ -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'
}
};