src: cache missing package.json files in the C++ package config cache

michaelsmithxyz · michaelsmithxyz · commit 9c5c99fed7dd · 2025-10-29T10:30:01.000-04:00
diff --git a/lib/internal/modules/package_json_reader.js b/lib/internal/modules/package_json_reader.js
@@ -5,11 +5,9 @@ const {
   JSONParse,
   ObjectDefineProperty,
   RegExpPrototypeExec,
-  SafeMap,
-  StringPrototypeEndsWith,
   StringPrototypeIndexOf,
-  StringPrototypeLastIndexOf,
   StringPrototypeSlice,
+  SafeMap,
 } = primordials;
 const {
   fileURLToPath,
@@ -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,
-      ...(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(plainExports) ? JSONParse(plainExports) : plainExports;
+        ObjectDefineProperty(data, 'exports', { __proto__: null, enumerable: true, value });
+        return value;
+      },
+    });
+  }
+
+  if (plainImports !== null) {
+    ObjectDefineProperty(data, 'imports', {
+      __proto__: null,
+      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,
   };
@@ -130,44 +139,8 @@ 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}
- */
-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);
-
-    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;
-}
+const moduleToParentPackageJSONCache = new SafeMap();
+const deserializedPackageJSONCache = new SafeMap();
 
 /**
  * Get the nearest parent package.json file from a given path.
@@ -176,26 +149,21 @@ function findParentPackageJSON(checkPath) {
  * @returns {undefined | DeserializedPackageConfig}
  */
 function getNearestParentPackageJSON(checkPath) {
-  const nearestParentPackageJSON = findParentPackageJSON(checkPath);
-
-  if (nearestParentPackageJSON === undefined) {
-    return undefined;
+  if (moduleToParentPackageJSONCache.has(checkPath)) {
+    const parentPackageJSONPath = moduleToParentPackageJSONCache.get(checkPath);
+    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;
+  if (deserializedPackageJSONCache.has(packageConfig.path)) {
+    return deserializedPackageJSONCache.get(packageConfig.path);
   }
 
-  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
@@ -98,15 +98,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.has_value()) {
+      return &cache_value.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 =
@@ -239,7 +251,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.value();
 }
 
 void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) {
@@ -320,6 +332,40 @@ const BindingData::PackageConfig* BindingData::TraverseParent(
   return nullptr;
 }
 
+void BindingData::GetNearestParentPackageJSON(
+    const v8::FunctionCallbackInfo<v8::Value>& 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);
+
+  std::string path_value_str = path_value.ToString();
+  if (slashCheck) {
+    path_value_str.push_back(kPathSeparator);
+  }
+
+  std::filesystem::path path;
+
+#ifdef _WIN32
+  std::wstring wide_path = ConvertToWideString(path_value_str, GetACP());
+  path = std::filesystem::path(wide_path);
+#else
+  path = std::filesystem::path(path_value_str);
+#endif
+
+  auto package_json = TraverseParent(realm, path);
+
+  if (package_json != nullptr) {
+    args.GetReturnValue().Set(package_json->Serialize(realm));
+  }
+}
+
 void BindingData::GetNearestParentPackageJSONType(
     const FunctionCallbackInfo<Value>& args) {
   CHECK_GE(args.Length(), 1);
@@ -646,6 +692,10 @@ void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data,
             target,
             "getNearestParentPackageJSONType",
             GetNearestParentPackageJSONType);
+  SetMethod(isolate,
+            target,
+            "getNearestParentPackageJSON",
+            GetNearestParentPackageJSON);
   SetMethod(
       isolate, target, "getPackageScopeConfig", GetPackageScopeConfig<false>);
   SetMethod(isolate, target, "getPackageType", GetPackageScopeConfig<true>);
@@ -702,6 +752,7 @@ void BindingData::RegisterExternalReferences(
     ExternalReferenceRegistry* registry) {
   registry->Register(ReadPackageJSON);
   registry->Register(GetNearestParentPackageJSONType);
+  registry->Register(GetNearestParentPackageJSON);
   registry->Register(GetPackageScopeConfig<false>);
   registry->Register(GetPackageScopeConfig<true>);
   registry->Register(EnableCompileCache);
diff --git 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<v8::Value>& args);
+  static void GetNearestParentPackageJSON(
+      const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetNearestParentPackageJSONType(
       const v8::FunctionCallbackInfo<v8::Value>& args);
   template <bool return_only_type>
@@ -72,7 +74,16 @@ class BindingData : public SnapshotableObject {
   static void RegisterExternalReferences(ExternalReferenceRegistry* registry);
 
  private:
-  std::unordered_map<std::string, PackageConfig> 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<
+    std::string,
+    std::optional<PackageConfig>
+  > package_configs_;
   simdjson::ondemand::parser json_parser;
   // returns null on error
   static const PackageConfig* GetPackageJSON(
diff --git 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 }
