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

Author: michaelsmithxyz

lib/internal/modules/package_json_reader.js

@@ -5,10 +5,7 @@ const {
   JSONParse,
   ObjectDefineProperty,
   RegExpPrototypeExec,
-  SafeMap,
-  StringPrototypeEndsWith,
   StringPrototypeIndexOf,
-  StringPrototypeLastIndexOf,
   StringPrototypeSlice,
 } = primordials;
 const {
@@ -28,11 +25,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
@@ -130,72 +125,15 @@ 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;
-}
-
 /**
  * Get the nearest parent package.json file from a given path.
  * Return the package.json data and the path to the package.json file, or undefined.
  * @param {string} checkPath The path to start searching from.
  * @returns {undefined | DeserializedPackageConfig}
  */
 function getNearestParentPackageJSON(checkPath) {
-  const nearestParentPackageJSON = findParentPackageJSON(checkPath);
-
-  if (nearestParentPackageJSON === undefined) {
-    return undefined;
-  }
-
-  if (nearestParentPackageJSONCache.has(nearestParentPackageJSON)) {
-    return nearestParentPackageJSONCache.get(nearestParentPackageJSON);
-  }
-
-  const result = modulesBinding.readPackageJSON(nearestParentPackageJSON);
-
-  if (result === undefined) {
-    nearestParentPackageJSONCache.set(checkPath, undefined);
-    return undefined;
-  }
-
+  const result = modulesBinding.getNearestParentPackageJSON(checkPath);
   const packageConfig = deserializePackageJSON(checkPath, result);
-  nearestParentPackageJSONCache.set(nearestParentPackageJSON, packageConfig);
-
   return packageConfig;
 }
 

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);

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(

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 }