src: improve package.json reader performance

nodejs · Jun 16, 2023 · fe6b124 · fe6b124
1 parent b85a2b1
commit fe6b124
Show file tree

Hide file tree

Showing 9 changed files with 220 additions and 180 deletions.
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -82,7 +82,6 @@ const {
   pendingDeprecate,
   emitExperimentalWarning,
   kEmptyObject,
-  filterOwnProperties,
   setOwnProperty,
   getLazy,
 } = require('internal/util');
@@ -353,36 +352,10 @@ function initializeCJS() {
 //   -> a.<ext>
 //   -> a/index.<ext>
 
-const packageJsonCache = new SafeMap();
-
 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 undefined or the filtered package.json as a JS object
+  return packageJsonReader.read(jsonPath);
 }
 
 let _readPackage = readPackage;

diff --git a/lib/internal/modules/esm/package_config.js b/lib/internal/modules/esm/package_config.js
@@ -1,18 +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
@@ -42,59 +34,23 @@ function getPackageConfig(path, specifier, base) {
     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 result = packageJsonReader.read(path);
+  const packageJSON = result ?? {
+    main: undefined,
+    name: undefined,
+    type: 'none',
+    exports: undefined,
+    imports: undefined,
+  };
 
-  const packageConfig = {
+  const json = {
+    __proto__: null,
     pjsonPath: path,
-    exists: true,
-    main,
-    name,
-    type,
-    exports,
-    imports,
+    exists: result !== undefined,
+    ...packageJSON,
   };
-  packageJSONCache.set(path, packageConfig);
-  return packageConfig;
+  packageJSONCache.set(path, json);
+  return json;
 }
 
 

diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
@@ -734,8 +734,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.name === packageName && packageConfig.exports !== undefined) {
       return packageExportsResolve(
         packageJSONUrl, packageSubpath, packageConfig, base, conditions);
     }
@@ -760,7 +759,7 @@ function packageResolve(specifier, base, conditions) {
 
     // Package match.
     const packageConfig = getPackageConfig(packageJSONPath, specifier, base);
-    if (packageConfig.exports !== undefined && packageConfig.exports !== null) {
+    if (packageConfig.exports !== undefined) {
       return packageExportsResolve(
         packageJSONUrl, packageSubpath, packageConfig, base, conditions);
     }

diff --git a/lib/internal/modules/package_json_reader.js b/lib/internal/modules/package_json_reader.js
@@ -1,6 +1,10 @@
 'use strict';
 
-const { SafeMap } = primordials;
+const {
+  JSONParse,
+  JSONStringify,
+  SafeMap,
+} = primordials;
 const { internalModuleReadJSON } = internalBinding('fs');
 const { pathToFileURL } = require('url');
 const { toNamespacedPath } = require('path');
@@ -10,30 +14,67 @@ const cache = new SafeMap();
 let manifest;
 
 /**
- *
+ * Returns undefined for all failure cases.
  * @param {string} jsonPath
+ * @returns {{
+ *   name?: string,
+ *   main?: string,
+ *   exports?: string | Record<string, unknown>,
+ *   imports?: string | Record<string, unknown>,
+ *   type: 'commonjs' | 'module' | 'none' | unknown,
+ * } | undefined}
  */
 function read(jsonPath) {
   if (cache.has(jsonPath)) {
     return cache.get(jsonPath);
   }
 
-  const { 0: string, 1: containsKeys } = internalModuleReadJSON(
+  const {
+    0: includesKeys,
+    1: name,
+    2: main,
+    3: exports,
+    4: imports,
+    5: type,
+    6: parseExports,
+    7: parseImports,
+  } = internalModuleReadJSON(
     toNamespacedPath(jsonPath),
   );
-  const result = { string, containsKeys };
-  const { getOptionValue } = require('internal/options');
-  if (string !== undefined) {
+
+  let result;
+
+  if (includesKeys !== undefined) {
+    result = {
+      __proto__: null,
+      name,
+      main,
+      exports,
+      imports,
+      type,
+    };
+
+    // Execute JSONParse on demand for improved performance
+    if (parseExports) {
+      result.exports = JSONParse(exports);
+    }
+
+    if (parseImports) {
+      result.imports = JSONParse(imports);
+    }
+
     if (manifest === undefined) {
+      const { getOptionValue } = require('internal/options');
       manifest = getOptionValue('--experimental-policy') ?
         require('internal/process/policy').manifest :
         null;
     }
     if (manifest !== null) {
       const jsonURL = pathToFileURL(jsonPath);
-      manifest.assertIntegrity(jsonURL, string);
+      manifest.assertIntegrity(jsonURL, JSONStringify(result));
     }
   }
+
   cache.set(jsonPath, result);
   return result;
 }

diff --git a/src/node_errors.h b/src/node_errors.h
@@ -66,6 +66,7 @@ void AppendExceptionLine(Environment* env,
   V(ERR_ILLEGAL_CONSTRUCTOR, Error)                                            \
   V(ERR_INVALID_ADDRESS, Error)                                                \
   V(ERR_INVALID_ARG_VALUE, TypeError)                                          \
+  V(ERR_INVALID_PACKAGE_CONFIG, SyntaxError)                                   \
   V(ERR_OSSL_EVP_INVALID_DIGEST, Error)                                        \
   V(ERR_INVALID_ARG_TYPE, TypeError)                                           \
   V(ERR_INVALID_OBJECT_DEFINE_PROPERTY, TypeError)                             \