feat: add normalize function

wraithgar · wraithgar · commit fbbf40173c85 · 2023-05-15T16:14:15.000-07:00
This brings in parity with `read-package-json-fast`, which is a
normalization method intended for parsing package.json files inside of
an existing node_modules.

Tests were copied straight from that package to ensure functionality was
compatible.  The only tests that weren't copied were the ones testing
down into the bin normalization. That is delegated to a subdependency so
these tests only ensure that *some* normalization is happening.
Eventually as we consolidate our package.json reading libs, bin
normalization can live in this package and be tested here.

Finally, the errors that this package was throwing now include the
metadata from the original errors (such as code) instead of making new
errors with no context.
diff --git a/README.md b/README.md
@@ -100,6 +100,20 @@ const pkgJson = await PackageJson.load('./')
 
 ---
 
+### `async PackageJson.normalize()`
+
+Like `load` but intended for reading package.json files in a
+node_modules tree.  Some light normalization is done to ensure that it
+is ready for use in `@npmcli/arborist`
+
+---
+
+### **static** `async PackageJson.normalize(path)`
+
+Convenience static method like `load` but for calling `normalize`
+
+---
+
 ### `PackageJson.update(content)`
 
 Updates the contents of the `package.json` with the `content` provided.
diff --git a/lib/index.js b/lib/index.js
@@ -1,18 +1,12 @@
-const fs = require('fs')
-const promisify = require('util').promisify
-const readFile = promisify(fs.readFile)
-const writeFile = promisify(fs.writeFile)
+const { readFile, writeFile } = require('fs/promises')
 const { resolve } = require('path')
 const updateDeps = require('./update-dependencies.js')
 const updateScripts = require('./update-scripts.js')
 const updateWorkspaces = require('./update-workspaces.js')
+const normalize = require('./normalize.js')
 
 const parseJSON = require('json-parse-even-better-errors')
 
-const _filename = Symbol('filename')
-const _manifest = Symbol('manifest')
-const _readFileContent = Symbol('readFileContent')
-
 // a list of handy specialized helper functions that take
 // care of special cases that are handled by the npm cli
 const knownSteps = new Set([
@@ -29,42 +23,54 @@ const knownKeys = new Set([
 ])
 
 class PackageJson {
+  // default behavior, just loads and parses
   static async load (path) {
     return await new PackageJson(path).load()
   }
 
+  // read-package-json-fast compatible behavior
+  static async normalize (path) {
+    return await new PackageJson(path).normalize()
+  }
+
+  #filename
+  #path
+  #manifest = {}
+  #readFileContent = ''
+
   constructor (path) {
-    this[_filename] = resolve(path, 'package.json')
-    this[_manifest] = {}
-    this[_readFileContent] = ''
+    this.#path = path
+    this.#filename = resolve(path, 'package.json')
   }
 
   async load () {
     try {
-      this[_readFileContent] =
-        await readFile(this[_filename], 'utf8')
+      this.#readFileContent =
+        await readFile(this.#filename, 'utf8')
     } catch (err) {
-      throw new Error('package.json not found')
+      err.message = `Could not read package.json: ${err}`
+      throw err
     }
 
     try {
-      this[_manifest] =
-        parseJSON(this[_readFileContent])
+      this.#manifest =
+        parseJSON(this.#readFileContent)
     } catch (err) {
-      throw new Error(`Invalid package.json: ${err}`)
+      err.message = `Invalid package.json: ${err}`
+      throw err
     }
 
     return this
   }
 
   get content () {
-    return this[_manifest]
+    return this.#manifest
   }
 
   update (content) {
     // validates both current manifest and content param
     const invalidContent =
-      typeof this[_manifest] !== 'object'
+      typeof this.#manifest !== 'object'
         || typeof content !== 'object'
     if (invalidContent) {
       throw Object.assign(
@@ -74,13 +80,13 @@ class PackageJson {
     }
 
     for (const step of knownSteps) {
-      this[_manifest] = step({ content, originalContent: this[_manifest] })
+      this.#manifest = step({ content, originalContent: this.#manifest })
     }
 
     // unknown properties will just be overwitten
     for (const [key, value] of Object.entries(content)) {
       if (!knownKeys.has(key)) {
-        this[_manifest][key] = value
+        this.#manifest[key] = value
       }
     }
 
@@ -91,19 +97,25 @@ class PackageJson {
     const {
       [Symbol.for('indent')]: indent,
       [Symbol.for('newline')]: newline,
-    } = this[_manifest]
+    } = this.#manifest
 
     const format = indent === undefined ? '  ' : indent
     const eol = newline === undefined ? '\n' : newline
     const fileContent = `${
-      JSON.stringify(this[_manifest], null, format)
+      JSON.stringify(this.#manifest, null, format)
     }\n`
       .replace(/\n/g, eol)
 
-    if (fileContent.trim() !== this[_readFileContent].trim()) {
-      return await writeFile(this[_filename], fileContent)
+    if (fileContent.trim() !== this.#readFileContent.trim()) {
+      return await writeFile(this.#filename, fileContent)
     }
   }
+
+  async normalize () {
+    await this.load()
+    await normalize(this)
+    return this
+  }
 }
 
 module.exports = PackageJson
diff --git a/lib/normalize.js b/lib/normalize.js
@@ -0,0 +1,68 @@
+const normalizePackageBin = require('npm-normalize-package-bin')
+
+const normalize = async (pkg) => {
+  const data = pkg.content
+
+  // remove _attributes
+  for (const key in data) {
+    if (key.startsWith('_')) {
+      delete pkg.content[key]
+    }
+  }
+
+  // _id
+  if (data.name && data.version) {
+    data._id = `${data.name}@${data.version}`
+  }
+
+  // bundleDependencies
+  if (data.bundleDependencies === undefined && data.bundledDependencies !== undefined) {
+    data.bundleDependencies = data.bundledDependencies
+  }
+  delete data.bundledDependencies
+  const bd = data.bundleDependencies
+  if (bd === true) {
+    data.bundleDependencies = Object.keys(data.dependencies || {})
+  } else if (bd && typeof bd === 'object') {
+    if (!Array.isArray(bd)) {
+      data.bundleDependencies = Object.keys(bd)
+    }
+  } else {
+    data.bundleDependencies = []
+  }
+
+  // it was once common practice to list deps both in optionalDependencies and
+  // in dependencies, to support npm versions that did not know about
+  // optionalDependencies.  This is no longer a relevant need, so duplicating
+  // the deps in two places is unnecessary and excessive.
+  if (data.dependencies &&
+    data.optionalDependencies && typeof data.optionalDependencies === 'object') {
+    for (const name in data.optionalDependencies) {
+      delete data.dependencies[name]
+    }
+    if (!Object.keys(data.dependencies).length) {
+      delete data.dependencies
+    }
+  }
+
+  // scripts
+  if (typeof data.scripts === 'object') {
+    for (const name in data.scripts) {
+      if (typeof data.scripts[name] !== 'string') {
+        delete data.scripts[name]
+      }
+    }
+  } else {
+    delete data.scripts
+  }
+
+  // funding
+  if (data.funding && typeof data.funding === 'string') {
+    data.funding = { url: data.funding }
+  }
+
+  // bin
+  normalizePackageBin(data)
+}
+
+module.exports = normalize
diff --git a/test/index.js b/test/index.js
@@ -89,8 +89,10 @@ t.test('read missing package.json', async t => {
   const path = t.testdirName
   return t.rejects(
     PackageJson.load(path),
-    /package.json not found/,
-    'should throw package.json not found error'
+    {
+      message: /package.json/,
+      code: 'ENOENT',
+    }
   )
 })
 
diff --git a/test/normalize.js b/test/normalize.js
