Skip to content

lib: merge cjs and esm package json reader caches #48477

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Jun 29, 2023
Merged

lib: merge cjs and esm package json reader caches #48477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d4e2a4e
lib: merge cjs and esm package json reader caches
anonrig Jun 16, 2023
b133e1c
test: add more tests
anonrig Jun 23, 2023
c769ca5
lib: add comment and todo by request
anonrig Jun 23, 2023
5ea3840
test: disable module loading error on aix
anonrig Jun 23, 2023
b4720bd
lib: revert changes for AIX
anonrig Jun 24, 2023
4d5ba8c
lib: change detection for aix and cjs
anonrig Jun 24, 2023
54dcf06
lib: fix worker bootstrap error
anonrig Jun 28, 2023
0ab48b6
lib: remove url from worker bootstrapped modules
anonrig Jun 28, 2023
a8fbf80
lib: move SafeSet declarations to internal/url
anonrig Jun 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 15 additions & 39 deletions lib/internal/modules/cjs/loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ const {
pendingDeprecate,
emitExperimentalWarning,
kEmptyObject,
filterOwnProperties,
setOwnProperty,
getLazy,
} = require('internal/util');
Expand Down Expand Up @@ -355,36 +354,12 @@ function initializeCJS() {
// -> a.<ext>
// -> a/index.<ext>

const packageJsonCache = new SafeMap();

/**
* @param {string} requestPath
* @return {PackageConfig}
*/
function readPackage(requestPath) {
const jsonPath = path.resolve(requestPath, 'package.json');

const existing = packageJsonCache.get(jsonPath);
if (existing !== undefined) return existing;

const result = packageJsonReader.read(jsonPath);
const json = result.containsKeys === false ? '{}' : result.string;
if (json === undefined) {
packageJsonCache.set(jsonPath, false);
return false;
}

try {
const filtered = filterOwnProperties(JSONParse(json), [
'name',
'main',
'exports',
'imports',
'type',
]);
packageJsonCache.set(jsonPath, filtered);
return filtered;
} catch (e) {
e.path = jsonPath;
e.message = 'Error parsing ' + jsonPath + ': ' + e.message;
throw e;
}
return packageJsonReader.read(path.resolve(requestPath, 'package.json'));
}

let _readPackage = readPackage;
Expand Down Expand Up @@ -414,7 +389,7 @@ function readPackageScope(checkPath) {
if (StringPrototypeEndsWith(checkPath, sep + 'node_modules'))
return false;
const pjson = _readPackage(checkPath + sep);
if (pjson) return {
if (pjson.exists) return {
data: pjson,
path: checkPath,
};
Expand All @@ -423,7 +398,7 @@ function readPackageScope(checkPath) {
}

function tryPackage(requestPath, exts, isMain, originalPath) {
const pkg = _readPackage(requestPath)?.main;
const pkg = _readPackage(requestPath).main;

if (!pkg) {
return tryExtensions(path.resolve(requestPath, 'index'), exts, isMain);
Expand Down Expand Up @@ -528,9 +503,10 @@ function trySelfParentPath(parent) {
function trySelf(parentPath, request) {
if (!parentPath) return false;

const { data: pkg, path: pkgPath } = readPackageScope(parentPath) || {};
if (!pkg || pkg.exports === undefined) return false;
if (typeof pkg.name !== 'string') return false;
const { data: pkg, path: pkgPath } = readPackageScope(parentPath);
if (!pkg || pkg.exports == null || pkg.name === undefined) {
return false;
}

let expansion;
if (request === pkg.name) {
Expand Down Expand Up @@ -565,7 +541,7 @@ function resolveExports(nmPath, request) {
return;
const pkgPath = path.resolve(nmPath, name);
const pkg = _readPackage(pkgPath);
if (pkg?.exports != null) {
if (pkg.exists && pkg.exports != null) {
try {
const { packageExportsResolve } = require('internal/modules/esm/resolve');
return finalizeEsmResolution(packageExportsResolve(
Expand Down Expand Up @@ -1028,7 +1004,7 @@ Module._resolveFilename = function(request, parent, isMain, options) {

if (request[0] === '#' && (parent?.filename || parent?.id === '<repl>')) {
const parentPath = parent?.filename ?? process.cwd() + path.sep;
const pkg = readPackageScope(parentPath) || {};
const pkg = readPackageScope(parentPath) || { __proto__: null };
if (pkg.data?.imports != null) {
try {
const { packageImportsResolve } = require('internal/modules/esm/resolve');
Expand Down Expand Up @@ -1274,9 +1250,9 @@ Module._extensions['.js'] = function(module, filename) {
content = fs.readFileSync(filename, 'utf8');
}
if (StringPrototypeEndsWith(filename, '.js')) {
const pkg = readPackageScope(filename);
const pkg = readPackageScope(filename) || { __proto__: null };
// Function require shouldn't be used in ES modules.
if (pkg?.data?.type === 'module') {
if (pkg.data?.type === 'module') {
const parent = moduleParentCache.get(module);
const parentPath = parent?.filename;
const packageJsonPath = path.resolve(pkg.path, 'package.json');
Expand Down
106 changes: 8 additions & 98 deletions lib/internal/modules/esm/package_config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,102 +1,10 @@
'use strict';

const {
JSONParse,
ObjectPrototypeHasOwnProperty,
SafeMap,
StringPrototypeEndsWith,
} = primordials;
const { URL, fileURLToPath } = require('internal/url');
const {
ERR_INVALID_PACKAGE_CONFIG,
} = require('internal/errors').codes;

const { filterOwnProperties } = require('internal/util');


/**
* @typedef {string | string[] | Record<string, unknown>} Exports
* @typedef {'module' | 'commonjs'} PackageType
* @typedef {{
* pjsonPath: string,
* exports?: ExportConfig,
* name?: string,
* main?: string,
* type?: PackageType,
* }} PackageConfig
*/

/** @type {Map<string, PackageConfig>} */
const packageJSONCache = new SafeMap();


/**
* @param {string} path
* @param {string} specifier
* @param {string | URL | undefined} base
* @returns {PackageConfig}
*/
function getPackageConfig(path, specifier, base) {
const existing = packageJSONCache.get(path);
if (existing !== undefined) {
return existing;
}
const packageJsonReader = require('internal/modules/package_json_reader');
const source = packageJsonReader.read(path).string;
if (source === undefined) {
const packageConfig = {
pjsonPath: path,
exists: false,
main: undefined,
name: undefined,
type: 'none',
exports: undefined,
imports: undefined,
};
packageJSONCache.set(path, packageConfig);
return packageConfig;
}

let packageJSON;
try {
packageJSON = JSONParse(source);
} catch (error) {
throw new ERR_INVALID_PACKAGE_CONFIG(
path,
(base ? `"${specifier}" from ` : '') + fileURLToPath(base || specifier),
error.message,
);
}

let { imports, main, name, type } = filterOwnProperties(packageJSON, ['imports', 'main', 'name', 'type']);
const exports = ObjectPrototypeHasOwnProperty(packageJSON, 'exports') ? packageJSON.exports : undefined;
if (typeof imports !== 'object' || imports === null) {
imports = undefined;
}
if (typeof main !== 'string') {
main = undefined;
}
if (typeof name !== 'string') {
name = undefined;
}
// Ignore unknown types for forwards compatibility
if (type !== 'module' && type !== 'commonjs') {
type = 'none';
}

const packageConfig = {
pjsonPath: path,
exists: true,
main,
name,
type,
exports,
imports,
};
packageJSONCache.set(path, packageConfig);
return packageConfig;
}

const packageJsonReader = require('internal/modules/package_json_reader');

/**
* @param {URL | string} resolved
Expand All @@ -109,7 +17,11 @@ function getPackageScopeConfig(resolved) {
if (StringPrototypeEndsWith(packageJSONPath, 'node_modules/package.json')) {
break;
}
const packageConfig = getPackageConfig(fileURLToPath(packageJSONUrl), resolved);
const packageConfig = packageJsonReader.read(fileURLToPath(packageJSONUrl), {
__proto__: null,
specifier: resolved,
isESM: true,
});
if (packageConfig.exists) {
return packageConfig;
}
Expand All @@ -124,7 +36,8 @@ function getPackageScopeConfig(resolved) {
}
}
const packageJSONPath = fileURLToPath(packageJSONUrl);
const packageConfig = {
return {
__proto__: null,
pjsonPath: packageJSONPath,
exists: false,
main: undefined,
Expand All @@ -133,12 +46,9 @@ function getPackageScopeConfig(resolved) {
exports: undefined,
imports: undefined,
};
packageJSONCache.set(packageJSONPath, packageConfig);
return packageConfig;
}


module.exports = {
getPackageConfig,
getPackageScopeConfig,
};
10 changes: 5 additions & 5 deletions lib/internal/modules/esm/resolve.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ const {
} = require('internal/errors').codes;

const { Module: CJSModule } = require('internal/modules/cjs/loader');
const { getPackageConfig, getPackageScopeConfig } = require('internal/modules/esm/package_config');
const { getPackageScopeConfig } = require('internal/modules/esm/package_config');
const { getConditionsSet } = require('internal/modules/esm/utils');
const packageJsonReader = require('internal/modules/package_json_reader');
const { internalModuleStat } = internalBinding('fs');

/**
Expand Down Expand Up @@ -734,8 +735,7 @@ function packageResolve(specifier, base, conditions) {
const packageConfig = getPackageScopeConfig(base);
if (packageConfig.exists) {
const packageJSONUrl = pathToFileURL(packageConfig.pjsonPath);
if (packageConfig.name === packageName &&
packageConfig.exports !== undefined && packageConfig.exports !== null) {
if (packageConfig.exports != null && packageConfig.name === packageName) {
return packageExportsResolve(
packageJSONUrl, packageSubpath, packageConfig, base, conditions);
}
Expand All @@ -759,8 +759,8 @@ function packageResolve(specifier, base, conditions) {
}

// Package match.
const packageConfig = getPackageConfig(packageJSONPath, specifier, base);
if (packageConfig.exports !== undefined && packageConfig.exports !== null) {
const packageConfig = packageJsonReader.read(packageJSONPath, { __proto__: null, specifier, base, isESM: true });
if (packageConfig.exports != null) {
return packageExportsResolve(
packageJSONUrl, packageSubpath, packageConfig, base, conditions);
}
Expand Down
Loading