Repository navigation
Improve legacy CJS resolve #73
Copy link
Copy link
Closed
Labels
good first issueGood for newcomersGood for newcomershelp wantedExtra attention is neededExtra attention is neededperformance-agenda
Description
The following code calls multiple C++ calls for each new URL and fileExists function call. We can avoid these changes, make 1 C++ call, and handle the rest in the C++ layer.
function legacyMainResolve(packageJSONUrl, packageConfig, base) {
let guess;
if (packageConfig.main !== undefined) {
// Note: fs check redundances will be handled by Descriptor cache here.
if (fileExists(guess = new URL(`./${packageConfig.main}`,
packageJSONUrl))) {
return guess;
} else if (fileExists(guess = new URL(`./${packageConfig.main}.js`,
packageJSONUrl)));
else if (fileExists(guess = new URL(`./${packageConfig.main}.json`,
packageJSONUrl)));
else if (fileExists(guess = new URL(`./${packageConfig.main}.node`,
packageJSONUrl)));
else if (fileExists(guess = new URL(`./${packageConfig.main}/index.js`,
packageJSONUrl)));
else if (fileExists(guess = new URL(`./${packageConfig.main}/index.json`,
packageJSONUrl)));
else if (fileExists(guess = new URL(`./${packageConfig.main}/index.node`,
packageJSONUrl)));
else guess = undefined;
if (guess) {
emitLegacyIndexDeprecation(guess, packageJSONUrl, base,
packageConfig.main);
return guess;
}
// Fallthrough.
}
if (fileExists(guess = new URL('./index.js', packageJSONUrl)));
// So fs.
else if (fileExists(guess = new URL('./index.json', packageJSONUrl)));
else if (fileExists(guess = new URL('./index.node', packageJSONUrl)));
else guess = undefined;
if (guess) {
emitLegacyIndexDeprecation(guess, packageJSONUrl, base, packageConfig.main);
return guess;
}
// Not found.
throw new ERR_MODULE_NOT_FOUND(
fileURLToPath(new URL('.', packageJSONUrl)), fileURLToPath(base));
}Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
good first issueGood for newcomersGood for newcomershelp wantedExtra attention is neededExtra attention is neededperformance-agenda
Activity
aduh95 commentedon Apr 13, 2023
Does it matter though? I'd expect that packages that do not define a
"main"nor"exports"field in theirpackage.jsonis rather rare – and maybe you could/should measure that before working on this.EDIT: I misread the code, there might still be lots of cases where packages define a
"main"but without an extension, in which case it might indeed be worth trying to optimize it.aduh95 commentedon Apr 13, 2023
Not related to performance, but another problem with that function is that it seems to assume that
"main"contains a URL-compatible string, when other part of the codebase (and the docs) treats it as a path.metcoder95 commentedon Apr 18, 2023
Then we have 2 separate issues?
Standardize its usage, and make optimizations out of it, right?
Not sure the former relates to performance but we can work at both ends, should this issue reflect that?
H4ad commentedon May 29, 2023
I started working on this issue, I will try more ideas and then share when the code is more mature.
H4ad commentedon Jun 3, 2023
Hey, I created a PR for NodeJS with current work: nodejs/node#48325
I'm almost done, the performance improvements have been really nice and also bring the possibility to improve other parts of the system like
fileURLToPath.