diff --git a/lib/internal/modules/package_json_reader.js b/lib/internal/modules/package_json_reader.js index af8fcd28c5259e..6c6bf0383bc338 100644 --- a/lib/internal/modules/package_json_reader.js +++ b/lib/internal/modules/package_json_reader.js @@ -6,9 +6,7 @@ const { ObjectDefineProperty, RegExpPrototypeExec, SafeMap, - StringPrototypeEndsWith, StringPrototypeIndexOf, - StringPrototypeLastIndexOf, StringPrototypeSlice, } = primordials; const { @@ -28,11 +26,9 @@ const { const { kEmptyObject } = require('internal/util'); const modulesBinding = internalBinding('modules'); const path = require('path'); -const permission = require('internal/process/permission'); const { validateString } = require('internal/validators'); const internalFsBinding = internalBinding('fs'); -const nearestParentPackageJSONCache = new SafeMap(); /** * @typedef {import('typings/internalBinding/modules').DeserializedPackageConfig} DeserializedPackageConfig @@ -68,28 +64,41 @@ function deserializePackageJSON(path, contents) { const pjsonPath = optionalFilePath ?? path; - return { - data: { + const data = { + __proto__: null, + ...(name != null && { name }), + ...(main != null && { main }), + ...(type != null && { type }), + }; + + if (plainExports !== null) { + ObjectDefineProperty(data, 'exports', { + __proto__: null, + configurable: true, + enumerable: true, + get() { + const value = requiresJSONParse(plainExports) ? JSONParse(plainExports) : plainExports; + ObjectDefineProperty(data, 'exports', { __proto__: null, enumerable: true, value }); + return value; + }, + }); + } + + if (plainImports !== null) { + ObjectDefineProperty(data, 'imports', { __proto__: null, - ...(name != null && { name }), - ...(main != null && { main }), - ...(type != null && { type }), - ...(plainImports != null && { - // This getters are used to lazily parse the imports and exports fields. - get imports() { - const value = requiresJSONParse(plainImports) ? JSONParse(plainImports) : plainImports; - ObjectDefineProperty(this, 'imports', { __proto__: null, value }); - return this.imports; - }, - }), - ...(plainExports != null && { - get exports() { - const value = requiresJSONParse(plainExports) ? JSONParse(plainExports) : plainExports; - ObjectDefineProperty(this, 'exports', { __proto__: null, value }); - return this.exports; - }, - }), - }, + configurable: true, + enumerable: true, + get() { + const value = requiresJSONParse(plainImports) ? JSONParse(plainImports) : plainImports; + ObjectDefineProperty(data, 'imports', { __proto__: null, enumerable: true, value }); + return value; + }, + }); + } + + return { + data, exists: true, path: pjsonPath, }; @@ -131,43 +140,23 @@ function read(jsonPath, { base, specifier, isESM } = kEmptyObject) { } /** - * Given a file path, walk the filesystem upwards until we find its closest parent - * `package.json` file, stopping when: - * 1. we find a `package.json` file; - * 2. we find a path that we do not have permission to read; - * 3. we find a containing `node_modules` directory; - * 4. or, we reach the filesystem root - * @returns {undefined | string} + * A cache mapping a module's path to its parent `package.json` file's path. + * This is used in concert with `deserializedPackageJSONCache` to improve + * the performance of `getNearestParentPackageJSON` when called repeatedly + * on the same module paths. */ -function findParentPackageJSON(checkPath) { - const enabledPermission = permission.isEnabled(); - - const rootSeparatorIndex = StringPrototypeIndexOf(checkPath, path.sep); - let separatorIndex; - - do { - separatorIndex = StringPrototypeLastIndexOf(checkPath, path.sep); - checkPath = StringPrototypeSlice(checkPath, 0, separatorIndex); +const moduleToParentPackageJSONCache = new SafeMap(); - if (enabledPermission && !permission.has('fs.read', checkPath + path.sep)) { - return undefined; - } - - if (StringPrototypeEndsWith(checkPath, path.sep + 'node_modules')) { - return undefined; - } - - const maybePackageJSONPath = checkPath + path.sep + 'package.json'; - const stat = internalFsBinding.internalModuleStat(checkPath + path.sep + 'package.json'); - - const packageJSONExists = stat === 0; - if (packageJSONExists) { - return maybePackageJSONPath; - } - } while (separatorIndex > rootSeparatorIndex); - - return undefined; -} +/** + * A cache mapping the path of a `package.json` file to its + * {@link DeserializedPackageConfig deserialized representation}, + * as produced by {@link deserializedPackageJSONCache}. The purpose of this + * cache is to ensure that we always return the same + * {@link DeserializedPackageConfig} instance for a given `package.json`, + * which is necessary to ensure that we don't re-parse `imports` and + * `exports` redundantly. + */ +const deserializedPackageJSONCache = new SafeMap(); /** * Get the nearest parent package.json file from a given path. @@ -176,26 +165,22 @@ function findParentPackageJSON(checkPath) { * @returns {undefined | DeserializedPackageConfig} */ function getNearestParentPackageJSON(checkPath) { - const nearestParentPackageJSON = findParentPackageJSON(checkPath); - - if (nearestParentPackageJSON === undefined) { - return undefined; + const parentPackageJSONPath = moduleToParentPackageJSONCache.get(checkPath); + if (parentPackageJSONPath !== undefined) { + return deserializedPackageJSONCache.get(parentPackageJSONPath); } - if (nearestParentPackageJSONCache.has(nearestParentPackageJSON)) { - return nearestParentPackageJSONCache.get(nearestParentPackageJSON); - } + const result = modulesBinding.getNearestParentPackageJSON(checkPath); + const packageConfig = deserializePackageJSON(checkPath, result); - const result = modulesBinding.readPackageJSON(nearestParentPackageJSON); + moduleToParentPackageJSONCache.set(checkPath, packageConfig.path); - if (result === undefined) { - nearestParentPackageJSONCache.set(checkPath, undefined); - return undefined; + const maybeCachedPackageConfig = deserializedPackageJSONCache.get(packageConfig.path); + if (maybeCachedPackageConfig !== undefined) { + return maybeCachedPackageConfig; } - const packageConfig = deserializePackageJSON(checkPath, result); - nearestParentPackageJSONCache.set(nearestParentPackageJSON, packageConfig); - + deserializedPackageJSONCache.set(packageConfig.path, packageConfig); return packageConfig; } diff --git a/src/node_modules.cc b/src/node_modules.cc index 9131c4f27e7d83..08177c5b3eb8b5 100644 --- a/src/node_modules.cc +++ b/src/node_modules.cc @@ -97,15 +97,27 @@ const BindingData::PackageConfig* BindingData::GetPackageJSON( auto cache_entry = binding_data->package_configs_.find(path.data()); if (cache_entry != binding_data->package_configs_.end()) { - return &cache_entry->second; + auto& cache_value = cache_entry->second; + if (cache_value) { + return &*cache_value; + } + + // If we have a cache entry without a value, we've already + // attempted to open and read this path and couldn't (it most + // likely doesn't exist) + return nullptr; } PackageConfig package_config{}; package_config.file_path = path; // No need to exclude BOM since simdjson will skip it. if (ReadFileSync(&package_config.raw_json, path.data()) < 0) { + // Add `nullopt` to the package config cache so that we don't + // need to open and attempt to read this path again + binding_data->package_configs_.insert({std::string(path), std::nullopt}); return nullptr; } + simdjson::ondemand::document document; simdjson::ondemand::object main_object; simdjson::error_code error = @@ -238,7 +250,7 @@ const BindingData::PackageConfig* BindingData::GetPackageJSON( auto cached = binding_data->package_configs_.insert( {std::string(path), std::move(package_config)}); - return &cached.first->second; + return &*cached.first->second; } void BindingData::ReadPackageJSON(const FunctionCallbackInfo& args) { @@ -321,24 +333,49 @@ const BindingData::PackageConfig* BindingData::TraverseParent( return nullptr; } -void BindingData::GetNearestParentPackageJSONType( - const FunctionCallbackInfo& args) { +const std::filesystem::path BindingData::NormalizePath( + Realm* realm, BufferValue* path_value) { + // Check if the path has a trailing slash. If so, add it after + // ToNamespacedPath() as it will be deleted by ToNamespacedPath() + bool slashCheck = path_value->ToStringView().ends_with(kPathSeparator); + + ToNamespacedPath(realm->env(), path_value); + + auto path = path_value->ToPath(); + + if (slashCheck) { + path /= ""; + } + + return path; +} + +void BindingData::GetNearestParentPackageJSON( + const v8::FunctionCallbackInfo& args) { CHECK_GE(args.Length(), 1); CHECK(args[0]->IsString()); Realm* realm = Realm::GetCurrent(args); BufferValue path_value(realm->isolate(), args[0]); - // Check if the path has a trailing slash. If so, add it after - // ToNamespacedPath() as it will be deleted by ToNamespacedPath() - bool slashCheck = path_value.ToStringView().ends_with(kPathSeparator); - ToNamespacedPath(realm->env(), &path_value); + auto path = NormalizePath(realm, &path_value); - auto path = path_value.ToPath(); + auto package_json = TraverseParent(realm, path); - if (slashCheck) { - path /= ""; + if (package_json != nullptr) { + args.GetReturnValue().Set(package_json->Serialize(realm)); } +} + +void BindingData::GetNearestParentPackageJSONType( + const FunctionCallbackInfo& args) { + CHECK_GE(args.Length(), 1); + CHECK(args[0]->IsString()); + + Realm* realm = Realm::GetCurrent(args); + BufferValue path_value(realm->isolate(), args[0]); + + auto path = NormalizePath(realm, &path_value); auto package_json = TraverseParent(realm, path); @@ -569,6 +606,10 @@ void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data, target, "getNearestParentPackageJSONType", GetNearestParentPackageJSONType); + SetMethod(isolate, + target, + "getNearestParentPackageJSON", + GetNearestParentPackageJSON); SetMethod( isolate, target, "getPackageScopeConfig", GetPackageScopeConfig); SetMethod(isolate, target, "getPackageType", GetPackageScopeConfig); @@ -624,6 +665,7 @@ void BindingData::RegisterExternalReferences( ExternalReferenceRegistry* registry) { registry->Register(ReadPackageJSON); registry->Register(GetNearestParentPackageJSONType); + registry->Register(GetNearestParentPackageJSON); registry->Register(GetPackageScopeConfig); registry->Register(GetPackageScopeConfig); registry->Register(EnableCompileCache); diff --git a/src/node_modules.h b/src/node_modules.h index e4ba6b75bc86d1..d610306a3a3111 100644 --- a/src/node_modules.h +++ b/src/node_modules.h @@ -55,6 +55,8 @@ class BindingData : public SnapshotableObject { SET_MEMORY_INFO_NAME(BindingData) static void ReadPackageJSON(const v8::FunctionCallbackInfo& args); + static void GetNearestParentPackageJSON( + const v8::FunctionCallbackInfo& args); static void GetNearestParentPackageJSONType( const v8::FunctionCallbackInfo& args); template @@ -72,8 +74,17 @@ class BindingData : public SnapshotableObject { static void RegisterExternalReferences(ExternalReferenceRegistry* registry); private: - std::unordered_map package_configs_; + /* + * This map caches `PackageConfig` values by `package.json` path. + * An empty optional value indicates that no `package.json` file + * at the given path exists, which we cache to avoid repeated + * attempts to open the same non-existent paths. + */ + std::unordered_map > + package_configs_; simdjson::ondemand::parser json_parser; + static const std::filesystem::path NormalizePath(Realm* realm, + BufferValue* path_value); // returns null on error static const PackageConfig* GetPackageJSON( Realm* realm, diff --git a/typings/internalBinding/modules.d.ts b/typings/internalBinding/modules.d.ts index c5a79ba9c2957e..0b1d0e2938319f 100644 --- a/typings/internalBinding/modules.d.ts +++ b/typings/internalBinding/modules.d.ts @@ -23,6 +23,7 @@ export type SerializedPackageConfig = [ export interface ModulesBinding { readPackageJSON(path: string): SerializedPackageConfig | undefined; getNearestParentPackageJSONType(path: string): PackageConfig['type'] + getNearestParentPackageJSON(path: string): SerializedPackageConfig | undefined getPackageScopeConfig(path: string): SerializedPackageConfig | undefined getPackageType(path: string): PackageConfig['type'] | undefined enableCompileCache(path?: string): { status: number, message?: string, directory?: string }