fix: use @npmcli/package-json

lib/dir.js

@@ -87,7 +87,7 @@ class DirFetcher extends Fetcher {
  return Promise.resolve(this.package)
  }
 
- return this[_readPackageJson](this.resolved + '/package.json')
+ return this[_readPackageJson](this.resolved)
  .then(mani => this.package = {
  ...mani,
  _integrity: this.integrity && String(this.integrity),

lib/fetcher.js

@@ -5,7 +5,6 @@
 
 const npa = require('npm-package-arg')
 const ssri = require('ssri')
-const { promisify } = require('util')
 const { basename, dirname } = require('path')
 const tar = require('tar')
 const { log } = require('proc-log')
@@ -16,12 +15,14 @@ const cacache = require('cacache')
 const isPackageBin = require('./util/is-package-bin.js')
 const removeTrailingSlashes = require('./util/trailing-slashes.js')
 const getContents = require('@npmcli/installed-package-contents')
-const readPackageJsonFast = require('read-package-json-fast')
-const readPackageJson = promisify(require('read-package-json'))
+const PackageJson = require('@npmcli/package-json')
 const { Minipass } = require('minipass')
-
 const cacheDir = require('./util/cache-dir.js')
 
+// Pacote is only concerned with the package.json contents
+const packageJsonPrepare = (p) => PackageJson.prepare(p).then(pkg => pkg.content)
+const packageJsonNormalize = (p) => PackageJson.normalize(p).then(pkg => pkg.content)
+
 // Private methods.
 // Child classes should not have to override these.
 // Users should never call them.
@@ -93,9 +94,9 @@ class FetcherBase {
  this.fullMetadata = this.before ? true : !!opts.fullMetadata
  this.fullReadJson = !!opts.fullReadJson
  if (this.fullReadJson) {
- this[_readPackageJson] = readPackageJson
+ this[_readPackageJson] = packageJsonPrepare
  } else {
- this[_readPackageJson] = readPackageJsonFast
+ this[_readPackageJson] = packageJsonNormalize
  }
 
  // rrh is a registry hostname or 'never' or 'always'

lib/file.js

@@ -1,10 +1,11 @@
-const Fetcher = require('./fetcher.js')
 const fsm = require('fs-minipass')
 const cacache = require('cacache')
-const _tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved')
-const _exeBins = Symbol('_exeBins')
 const { resolve } = require('path')
-const fs = require('fs')
+const { stat, chmod } = require('fs/promises')
+const Fetcher = require('./fetcher.js')
+
+const _exeBins = Symbol('_exeBins')
+const _tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved')
 const _readPackageJson = Symbol.for('package.Fetcher._readPackageJson')
 
 class FileFetcher extends Fetcher {
@@ -26,7 +27,7 @@ class FileFetcher extends Fetcher {
  // have to unpack the tarball for this.
  return cacache.tmp.withTmp(this.cache, this.opts, dir =>
  this.extract(dir)
- .then(() => this[_readPackageJson](dir + '/package.json'))
+ .then(() => this[_readPackageJson](dir))
  .then(mani => this.package = {
  ...mani,
  _integrity: this.integrity && String(this.integrity),
@@ -40,31 +41,31 @@ class FileFetcher extends Fetcher {
  return Promise.resolve()
  }
 
- return Promise.all(Object.keys(pkg.bin).map(k => new Promise(res => {
+ return Promise.all(Object.keys(pkg.bin).map(async k => {
  const script = resolve(dest, pkg.bin[k])
  // Best effort. Ignore errors here, the only result is that
  // a bin script is not executable. But if it's missing or
  // something, we just leave it for a later stage to trip over
  // when we can provide a more useful contextual error.
- fs.stat(script, (er, st) => {
- if (er) {
- return res()
- }
+ try {
+ const st = await stat(script)
  const mode = st.mode | 0o111
  if (mode === st.mode) {
- return res()
+ return
  }
- fs.chmod(script, mode, res)
- })
- })))
+ await chmod(script, mode)
+ } catch {
+ // Ignore errors here
+ }
+ }))
  }
 
  extract (dest) {
  // if we've already loaded the manifest, then the super got it.
  // but if not, read the unpacked manifest and chmod properly.
  return super.extract(dest)
  .then(result => this.package ? result
- : this[_readPackageJson](dest + '/package.json').then(pkg =>
+ : this[_readPackageJson](dest).then(pkg =>
  this[_exeBins](pkg, dest)).then(() => result))
  }
 

lib/git.js

@@ -156,11 +156,11 @@ class GitFetcher extends Fetcher {
  [_resolvedFromClone] () {
  // do a full or shallow clone, then look at the HEAD
  // kind of wasteful, but no other option, really
- return this[_clone](dir => this.resolved)
+ return this[_clone](() => this.resolved)
  }
 
  [_prepareDir] (dir) {
- return this[_readPackageJson](dir + '/package.json').then(mani => {
+ return this[_readPackageJson](dir).then(mani => {
  // no need if we aren't going to do any preparation.
  const scripts = mani.scripts
  if (!mani.workspaces && (!scripts || !(
@@ -312,7 +312,7 @@ class GitFetcher extends Fetcher {
  return this.spec.hosted && this.resolved
  ? FileFetcher.prototype.manifest.apply(this)
  : this[_clone](dir =>
- this[_readPackageJson](dir + '/package.json')
+ this[_readPackageJson](dir)
  .then(mani => this.package = {
  ...mani,
  _resolved: this.resolved,

lib/registry.js

@@ -3,7 +3,7 @@ const RemoteFetcher = require('./remote.js')
 const _tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved')
 const pacoteVersion = require('../package.json').version
 const removeTrailingSlashes = require('./util/trailing-slashes.js')
-const rpj = require('read-package-json-fast')
+const PackageJson = require('@npmcli/package-json')
 const pickManifest = require('npm-pick-manifest')
 const ssri = require('ssri')
 const crypto = require('crypto')
@@ -127,12 +127,12 @@ class RegistryFetcher extends Fetcher {
  }
 
  const packument = await this.packument()
- let mani = await pickManifest(packument, this.spec.fetchSpec, {
+ const mani = await new PackageJson().fromContent(pickManifest(packument, this.spec.fetchSpec, {
  ...this.opts,
  defaultTag: this.defaultTag,
  before: this.before,
- })
- mani = rpj.normalize(mani)
+ })).normalize().then(p => p.content)
+
  /* XXX add ETARGET and E403 revalidation of cached packuments here */
 
  // add _time from packument if fetched with fullMetadata

package.json

@@ -46,6 +46,7 @@
  "dependencies": {
  "@npmcli/git": "^5.0.0",
  "@npmcli/installed-package-contents": "^2.0.1",
+ "@npmcli/package-json": "^5.1.0",
  "@npmcli/promise-spawn": "^7.0.0",
  "@npmcli/run-script": "^8.0.0",
  "cacache": "^18.0.0",
@@ -57,8 +58,6 @@
  "npm-registry-fetch": "^16.0.0",
  "proc-log": "^4.0.0",
  "promise-retry": "^2.0.1",
- "read-package-json": "^7.0.0",
- "read-package-json-fast": "^3.0.0",
  "sigstore": "^2.2.0",
  "ssri": "^10.0.0",
  "tar": "^6.1.11"

test/bin.js

@@ -23,7 +23,7 @@ pacote.manifest = (spec, conf) => Promise.resolve({
 pacote.packument = (spec, conf) => Promise.resolve({ method: 'packument', spec, conf })
 pacote.tarball.file = (spec, file, conf) => Promise.resolve({ method: 'tarball', spec, file, conf })
 const { Minipass } = require('minipass')
-pacote.tarball.stream = (spec, handler, conf) => handler(new Minipass().end('tarball data'))
+pacote.tarball.stream = (spec, handler) => handler(new Minipass().end('tarball data'))
 pacote.extract = (spec, dest, conf) => Promise.resolve({ method: 'extract', spec, dest, conf })
 
 t.test('running bin runs main file', t => {

test/fetcher.js

@@ -94,15 +94,14 @@ t.test('tarball data', async t => {
 
 t.test('tarballFile', t => {
  const target = resolve(me, 'tarball-file')
- t.test('basic copy', t =>
- new FileFetcher(abbrevspec, { cache })
- .tarballFile(target + '/basic/1.tgz'))
 
- t.test('again, folder already created', t =>
- new FileFetcher(abbrevspec, { cache })
- .tarballFile(target + '/basic/2.tgz'))
+ t.test('basic', async t => {
+ await new FileFetcher(abbrevspec, { cache })
+ .tarballFile(target + '/basic/1.tgz')
+
+ await new FileFetcher(abbrevspec, { cache })
+ .tarballFile(target + '/basic/2.tgz')
 
- t.test('check it', t => {
  const one = fs.readFileSync(target + '/basic/1.tgz')
  const two = fs.readFileSync(target + '/basic/2.tgz')
  const expect = fs.readFileSync(abbrev)

test/git.js

@@ -87,7 +87,7 @@ const tar = require('tar')
 
 let REPO_HEAD = ''
 t.test('setup', { bail: true }, t => {
- t.test('create repo', t => {
+ t.test('create repo', () => {
  const git = (...cmd) => spawnGit(cmd, { cwd: repo })
  const write = (f, c) => fs.writeFileSync(`${repo}/${f}`, c)
  const npm = (...cmd) => spawnNpm('npm', [
@@ -162,7 +162,7 @@ t.test('setup', { bail: true }, t => {
  .then(({ stdout }) => REPO_HEAD = stdout.trim())
  })
 
- t.test('create cycle of git prepared deps', async t => {
+ t.test('create cycle of git prepared deps', async () => {
  for (const repoDir of [cycleA, cycleB]) {
  const git = (...cmd) => spawnGit(cmd, { cwd: repoDir })
  const write = (f, c) => fs.writeFileSync(`${repoDir}/${f}`, c)
@@ -227,7 +227,7 @@ t.test('setup', { bail: true }, t => {
  daemon.on('close', () => rmSync(me, { recursive: true, force: true }))
  })
 
- t.test('create a repo with a submodule', t => {
+ t.test('create a repo with a submodule', () => {
  const submoduleRepo = resolve(me, 'submodule-repo')
  const git = (...cmd) => spawnGit(cmd, { cwd: submoduleRepo })
  const write = (f, c) => fs.writeFileSync(`${submoduleRepo}/${f}`, c)
@@ -285,7 +285,7 @@ t.test('setup', { bail: true }, t => {
  .then(() => git('commit', '-m', 'package'))
  })
 
- t.test('create a repo with workspaces', t => {
+ t.test('create a repo with workspaces', () => {
  const workspacesRepo = resolve(me, 'workspaces-repo')
  const wsfolder = resolve(me, 'workspaces-repo/a')
  const git = (...cmd) => spawnGit(cmd, { cwd: workspacesRepo })
@@ -315,7 +315,7 @@ t.test('setup', { bail: true }, t => {
  .then(() => git('commit', '-m', 'a/package.json'))
  })
 
- t.test('create a repo with only a prepack script', t => {
+ t.test('create a repo with only a prepack script', () => {
  const prepackRepo = resolve(me, 'prepack-repo')
  const git = (...cmd) => spawnGit(cmd, { cwd: prepackRepo })
  const write = (f, c) => fs.writeFileSync(`${prepackRepo}/${f}`, c)
@@ -608,7 +608,7 @@ t.test('fetch a weird ref', t => {
  t.end()
 })
 
-t.test('fetch a private repo where the tgz is a 404', t => {
+t.test('fetch a private repo where the tgz is a 404', () => {
  const gf = new GitFetcher(`localhost:repo/x#${REPO_HEAD}`, opts)
  gf.spec.hosted.tarball = () => `${hostedUrl}/not-found.tgz`
  // should fetch it by falling back to ssh when it gets an http error