fix: use @npmcli/package-json to parse package.json

Author: wraithgar

workspaces/arborist/lib/arborist/build-ideal-tree.js

@@ -1,6 +1,6 @@
 // mixin implementing the buildIdealTree method
 const localeCompare = require('@isaacs/string-locale-compare')('en')
-const rpj = require('read-package-json-fast')
+const PackageJson = require('@npmcli/package-json')
 const npa = require('npm-package-arg')
 const pacote = require('pacote')
 const cacache = require('cacache')
@@ -268,7 +268,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
       root = await this.#globalRootNode()
     } else {
       try {
-        const pkg = await rpj(this.path + '/package.json')
+        const { content: pkg } = await PackageJson.normalize(this.path)
         root = await this.#rootNodeFromPackage(pkg)
       } catch (err) {
         if (err.code === 'EJSONPARSE') {
@@ -448,7 +448,6 @@ module.exports = cls => class IdealTreeBuilder extends cls {
       const paths = await readdirScoped(nm).catch(() => [])
       for (const p of paths) {
         const name = p.replace(/\\/g, '/')
-        tree.package.dependencies = tree.package.dependencies || {}
         const updateName = this[_updateNames].includes(name)
         if (this[_updateAll] || updateName) {
           if (updateName) {
@@ -1288,14 +1287,13 @@ This is a one-time fix-up, please be patient...
       })
   }
 
-  #linkFromSpec (name, spec, parent) {
+  async #linkFromSpec (name, spec, parent) {
     const realpath = spec.fetchSpec
     const { installLinks, legacyPeerDeps } = this
-    return rpj(realpath + '/package.json').catch(() => ({})).then(pkg => {
-      const link = new Link({ name, parent, realpath, pkg, installLinks, legacyPeerDeps })
-      this.#linkNodes.add(link)
-      return link
-    })
+    const { content: pkg } = await PackageJson.normalize(realpath).catch(() => { return { content: {} } })
+    const link = new Link({ name, parent, realpath, pkg, installLinks, legacyPeerDeps })
+    this.#linkNodes.add(link)
+    return link
   }
 
   // load all peer deps and meta-peer deps into the node's parent

workspaces/arborist/lib/arborist/load-actual.js

@@ -2,7 +2,7 @@
 
 const { relative, dirname, resolve, join, normalize } = require('node:path')
 
-const rpj = require('read-package-json-fast')
+const PackageJson = require('@npmcli/package-json')
 const { readdirScoped } = require('@npmcli/fs')
 const { walkUp } = require('walk-up-path')
 const ancestorPath = require('common-ancestor-path')
@@ -279,7 +279,7 @@ module.exports = cls => class ActualLoader extends cls {
       }
 
       try {
-        const pkg = await rpj(join(real, 'package.json'))
+        const { content: pkg } = await PackageJson.normalize(real)
         params.pkg = pkg
         if (useRootOverrides && root.overrides) {
           params.overrides = root.overrides.getNodeRule({ name: pkg.name, version: pkg.version })

workspaces/arborist/lib/arborist/load-virtual.js

@@ -1,16 +1,15 @@
+const { resolve } = require('node:path')
 // mixin providing the loadVirtual method
 const mapWorkspaces = require('@npmcli/map-workspaces')
-
-const { resolve } = require('node:path')
-
+const PackageJson = require('@npmcli/package-json')
 const nameFromFolder = require('@npmcli/name-from-folder')
+
 const consistentResolve = require('../consistent-resolve.js')
 const Shrinkwrap = require('../shrinkwrap.js')
 const Node = require('../node.js')
 const Link = require('../link.js')
 const relpath = require('../relpath.js')
 const calcDepFlags = require('../calc-dep-flags.js')
-const rpj = require('read-package-json-fast')
 const treeCheck = require('../tree-check.js')
 
 const flagsSuspect = Symbol.for('flagsSuspect')
@@ -54,23 +53,18 @@ module.exports = cls => class VirtualLoader extends cls {
 
     // when building the ideal tree, we pass in a root node to this function
     // otherwise, load it from the root package json or the lockfile
+    const pkg = await PackageJson.normalize(this.path).then(p => p.content).catch(() => s.data.packages[''] || {})
+    // TODO clean this up
     const {
-      root = await this.#loadRoot(s),
+      root = await this[setWorkspaces](this.#loadNode('', pkg, true))
     } = options
-
     this.#rootOptionProvided = options.root
 
     await this.#loadFromShrinkwrap(s, root)
     root.assertRootOverrides()
     return treeCheck(this.virtualTree)
   }
 
-  async #loadRoot (s) {
-    const pj = this.path + '/package.json'
-    const pkg = await rpj(pj).catch(() => s.data.packages['']) || {}
-    return this[setWorkspaces](this.#loadNode('', pkg, true))
-  }
-
   async #loadFromShrinkwrap (s, root) {
     if (!this.#rootOptionProvided) {
       // root is never any of these things, but might be a brand new
@@ -219,11 +213,7 @@ To fix:
       // we always need to read the package.json for link targets
       // outside node_modules because they can be changed by the local user
       if (!link.target.parent) {
-        const pj = link.realpath + '/package.json'
-        const pkg = await rpj(pj).catch(() => null)
-        if (pkg) {
-          link.target.package = pkg
-        }
+        await PackageJson.normalize(link.realpath).then(p => link.target.package = p.content).catch(() => null)
       }
     }
   }

workspaces/arborist/lib/arborist/rebuild.js

@@ -1,20 +1,19 @@
 // Arborist.rebuild({path = this.path}) will do all the binlinks and
 // bundle building needed.  Called by reify, and by `npm rebuild`.
 
+const PackageJson = require('@npmcli/package-json')
+const binLinks = require('bin-links')
 const localeCompare = require('@isaacs/string-locale-compare')('en')
-const { depth: dfwalk } = require('treeverse')
 const promiseAllRejectLate = require('promise-all-reject-late')
-const rpj = require('read-package-json-fast')
-const binLinks = require('bin-links')
 const runScript = require('@npmcli/run-script')
 const { callLimit: promiseCallLimit } = require('promise-call-limit')
-const { resolve } = require('node:path')
+const { depth: dfwalk } = require('treeverse')
 const { isNodeGypPackage, defaultGypInstallScript } = require('@npmcli/node-gyp')
 const { log, time } = require('proc-log')
+const { resolve } = require('node:path')
 
 const boolEnv = b => b ? '1' : ''
-const sortNodes = (a, b) =>
-  (a.depth - b.depth) || localeCompare(a.path, b.path)
+const sortNodes = (a, b) => (a.depth - b.depth) || localeCompare(a.path, b.path)
 
 const _checkBins = Symbol.for('checkBins')
 
@@ -250,7 +249,7 @@ module.exports = cls => class Builder extends cls {
       // add to the set then remove while we're reading the pj, so we
       // don't accidentally hit it multiple times.
       set.add(node)
-      const pkg = await rpj(node.path + '/package.json').catch(() => ({}))
+      const { content: pkg } = await PackageJson.normalize(node.path).catch(() => { return { content: {} } })
       set.delete(node)
 
       const { scripts = {} } = pkg

workspaces/arborist/lib/arborist/reify.js

@@ -1,43 +1,33 @@
 // mixin implementing the reify method
-const onExit = require('../signal-handling.js')
-const pacote = require('pacote')
-const AuditReport = require('../audit-report.js')
-const { subset, intersects } = require('semver')
-const npa = require('npm-package-arg')
-const semver = require('semver')
-const debug = require('../debug.js')
-const { walkUp } = require('walk-up-path')
-const { log, time } = require('proc-log')
-const rpj = require('read-package-json-fast')
-const hgi = require('hosted-git-info')
-
-const { dirname, resolve, relative, join } = require('node:path')
-const { depth: dfwalk } = require('treeverse')
-const {
-  lstat,
-  mkdir,
-  rm,
-  symlink,
-} = require('node:fs/promises')
-const { moveFile } = require('@npmcli/fs')
 const PackageJson = require('@npmcli/package-json')
+const hgi = require('hosted-git-info')
+const npa = require('npm-package-arg')
 const packageContents = require('@npmcli/installed-package-contents')
+const pacote = require('pacote')
+const promiseAllRejectLate = require('promise-all-reject-late')
 const runScript = require('@npmcli/run-script')
+const { callLimit: promiseCallLimit } = require('promise-call-limit')
 const { checkEngine, checkPlatform } = require('npm-install-checks')
+const { depth: dfwalk } = require('treeverse')
+const { dirname, resolve, relative, join } = require('node:path')
+const { log, time } = require('proc-log')
+const { lstat, mkdir, rm, symlink, } = require('node:fs/promises')
+const { moveFile } = require('@npmcli/fs')
+const { subset, intersects } = require('semver')
+const { walkUp } = require('walk-up-path')
 
-const treeCheck = require('../tree-check.js')
-const relpath = require('../relpath.js')
+const AuditReport = require('../audit-report.js')
 const Diff = require('../diff.js')
-const retirePath = require('../retire-path.js')
-const promiseAllRejectLate = require('promise-all-reject-late')
-const { callLimit: promiseCallLimit } = require('promise-call-limit')
-const optionalSet = require('../optional-set.js')
 const calcDepFlags = require('../calc-dep-flags.js')
+const debug = require('../debug.js')
+const onExit = require('../signal-handling.js')
+const optionalSet = require('../optional-set.js')
+const relpath = require('../relpath.js')
+const retirePath = require('../retire-path.js')
+const treeCheck = require('../tree-check.js')
+const { defaultLockfileVersion } = require('../shrinkwrap.js')
 const { saveTypeMap, hasSubKey } = require('../add-rm-pkg-deps.js')
 
-const Shrinkwrap = require('../shrinkwrap.js')
-const { defaultLockfileVersion } = Shrinkwrap
-
 // Part of steps (steps need refactoring before we can do anything about these)
 const _retireShallowNodes = Symbol.for('retireShallowNodes')
 const _loadBundlesAndUpdateTrees = Symbol.for('loadBundlesAndUpdateTrees')
@@ -803,7 +793,7 @@ module.exports = cls => class Reifier extends cls {
       })
       // store nodes don't use Node class so node.package doesn't get updated
       if (node.isInStore) {
-        const pkg = await rpj(join(node.path, 'package.json'))
+        const { content: pkg } = await PackageJson.normalize(node.path)
         node.package.scripts = pkg.scripts
       }
       return
@@ -1432,8 +1422,7 @@ module.exports = cls => class Reifier extends cls {
         if (options.saveType) {
           const depType = saveTypeMap.get(options.saveType)
           pkg[depType][name] = newSpec
-          // rpj will have moved it here if it was in both
-          // if it is empty it will be deleted later
+          // PackageJson.normalize will have moved it here if it was in both, if it is empty it will be deleted later
           if (options.saveType === 'prod' && pkg.optionalDependencies) {
             delete pkg.optionalDependencies[name]
           }
@@ -1474,7 +1463,7 @@ module.exports = cls => class Reifier extends cls {
     const exactVersion = node => {
       for (const edge of node.edgesIn) {
         try {
-          if (semver.subset(edge.spec, node.version)) {
+          if (subset(edge.spec, node.version)) {
             return false
           }
         } catch {

workspaces/arborist/lib/node.js

@@ -28,22 +28,28 @@
 // where we need to quickly find all instances of a given package name within a
 // tree.
 
-const semver = require('semver')
+const PackageJson = require('@npmcli/package-json')
 const nameFromFolder = require('@npmcli/name-from-folder')
+const npa = require('npm-package-arg')
+const semver = require('semver')
+const util = require('node:util')
+const { getPaths: getBinPaths } = require('bin-links')
+const { log } = require('proc-log')
+const { resolve, relative, dirname, basename } = require('node:path')
+const { walkUp } = require('walk-up-path')
+
+const CaseInsensitiveMap = require('./case-insensitive-map.js')
 const Edge = require('./edge.js')
 const Inventory = require('./inventory.js')
 const OverrideSet = require('./override-set.js')
-const { normalize } = require('read-package-json-fast')
-const { getPaths: getBinPaths } = require('bin-links')
-const npa = require('npm-package-arg')
+const consistentResolve = require('./consistent-resolve.js')
 const debug = require('./debug.js')
 const gatherDepSet = require('./gather-dep-set.js')
+const printableTree = require('./printable.js')
+const querySelectorAll = require('./query-selector-all.js')
+const relpath = require('./relpath.js')
 const treeCheck = require('./tree-check.js')
-const { walkUp } = require('walk-up-path')
-const { log } = require('proc-log')
 
-const { resolve, relative, dirname, basename } = require('node:path')
-const util = require('node:util')
 const _package = Symbol('_package')
 const _parent = Symbol('_parent')
 const _target = Symbol.for('_target')
@@ -58,13 +64,6 @@ const _delistFromMeta = Symbol.for('_delistFromMeta')
 const _explain = Symbol('_explain')
 const _explanation = Symbol('_explanation')
 
-const relpath = require('./relpath.js')
-const consistentResolve = require('./consistent-resolve.js')
-
-const printableTree = require('./printable.js')
-const CaseInsensitiveMap = require('./case-insensitive-map.js')
-
-const querySelectorAll = require('./query-selector-all.js')
 
 class Node {
   #global
@@ -121,14 +120,25 @@ class Node {
     // package's dependencies in a virtual root.
     this.sourceReference = sourceReference
 
-    // TODO if this came from pacote.manifest we don't have to do this,
-    // we can be told to skip this step
-    const pkg = sourceReference ? sourceReference.package
-      : normalize(options.pkg || {})
+    // have to set the internal package ref before assigning the parent, because this.package is read when adding to inventory
+    if (sourceReference) {
+      this[_package] = sourceReference.package
+    } else {
+      // TODO if this came from pacote.manifest we don't have to do this, we can be told to skip this step
+      const pkg = new PackageJson()
+      let content = {}
+      // TODO this is overly guarded.  If pkg is not an object we should not allow it at all.
+      if (options.pkg && typeof options.pkg === 'object') {
+        content = options.pkg
+      }
+      pkg.fromContent(content)
+      pkg.syncNormalize()
+      this[_package] = pkg.content
+    }
 
     this.name = name ||
-      nameFromFolder(path || pkg.name || realpath) ||
-      pkg.name ||
+      nameFromFolder(path || this.package.name || realpath) ||
+      this.package.name ||
       null
 
     // should be equal if not a link
@@ -156,13 +166,13 @@ class Node {
       // probably what we're getting from pacote, which IS trustworthy.
       //
       // Otherwise, hopefully a shrinkwrap will help us out.
-      const resolved = consistentResolve(pkg._resolved)
-      if (resolved && !(/^file:/.test(resolved) && pkg._where)) {
+      const resolved = consistentResolve(this.package._resolved)
+      if (resolved && !(/^file:/.test(resolved) && this.package._where)) {
         this.resolved = resolved
       }
     }
-    this.integrity = integrity || pkg._integrity || null
-    this.hasShrinkwrap = hasShrinkwrap || pkg._hasShrinkwrap || false
+    this.integrity = integrity || this.package._integrity || null
+    this.hasShrinkwrap = hasShrinkwrap || this.package._hasShrinkwrap || false
     this.installLinks = installLinks
     this.legacyPeerDeps = legacyPeerDeps
 
@@ -203,17 +213,13 @@ class Node {
     this.edgesIn = new Set()
     this.edgesOut = new CaseInsensitiveMap()
 
-    // have to set the internal package ref before assigning the parent,
-    // because this.package is read when adding to inventory
-    this[_package] = pkg && typeof pkg === 'object' ? pkg : {}
-
     if (overrides) {
       this.overrides = overrides
     } else if (loadOverrides) {
-      const overrides = this[_package].overrides || {}
+      const overrides = this.package.overrides || {}
       if (Object.keys(overrides).length > 0) {
         this.overrides = new OverrideSet({
-          overrides: this[_package].overrides,
+          overrides: this.package.overrides,
         })
       }
     }
@@ -314,7 +320,7 @@ class Node {
     }
 
     return getBinPaths({
-      pkg: this[_package],
+      pkg: this.package,
       path: this.path,
       global: this.global,
       top: this.globalTop,
@@ -328,11 +334,11 @@ class Node {
   }
 
   get version () {
-    return this[_package].version || ''
+    return this.package.version || ''
   }
 
   get packageName () {
-    return this[_package].name || null
+    return this.package.name || null
   }
 
   get pkgid () {