fix: clean up requires (#371)

Missed a totally private symbol, moved to private method
added `node:` prefix to requires
cleaned up a few tests that needed it to work w/ the new requires

lib/dir.js

@@ -1,12 +1,12 @@
-const Fetcher = require('./fetcher.js')
-const FileFetcher = require('./file.js')
-const { Minipass } = require('minipass')
-const tarCreateOptions = require('./util/tar-create-options.js')
+const { resolve } = require('node:path')
 const packlist = require('npm-packlist')
-const tar = require('tar')
-const { resolve } = require('path')
 const runScript = require('@npmcli/run-script')
+const tar = require('tar')
+const { Minipass } = require('minipass')
+const Fetcher = require('./fetcher.js')
+const FileFetcher = require('./file.js')
 const _ = require('./util/protected.js')
+const tarCreateOptions = require('./util/tar-create-options.js')
 
 class DirFetcher extends Fetcher {
   constructor (spec, opts) {
@@ -27,7 +27,7 @@ class DirFetcher extends Fetcher {
     return ['directory']
   }
 
-  [_.prepareDir] () {
+  #prepareDir () {
     return this.manifest().then(mani => {
       if (!mani.scripts || !mani.scripts.prepare) {
         return
@@ -65,7 +65,7 @@ class DirFetcher extends Fetcher {
 
     // run the prepare script, get the list of files, and tar it up
     // pipe to the stream, and proxy errors the chain.
-    this[_.prepareDir]()
+    this.#prepareDir()
       .then(async () => {
         if (!this.tree) {
           const arb = new this.Arborist({ path: this.resolved })

lib/fetcher.js

@@ -3,22 +3,22 @@
 // It handles the unpacking and retry logic that is shared among
 // all of the other Fetcher types.
 
+const { basename, dirname } = require('node:path')
+const { rm, mkdir } = require('node:fs/promises')
+const PackageJson = require('@npmcli/package-json')
+const cacache = require('cacache')
+const fsm = require('fs-minipass')
+const getContents = require('@npmcli/installed-package-contents')
 const npa = require('npm-package-arg')
+const retry = require('promise-retry')
 const ssri = require('ssri')
-const { basename, dirname } = require('path')
 const tar = require('tar')
+const { Minipass } = require('minipass')
 const { log } = require('proc-log')
-const retry = require('promise-retry')
-const fs = require('fs/promises')
-const fsm = require('fs-minipass')
-const cacache = require('cacache')
+const _ = require('./util/protected.js')
+const cacheDir = require('./util/cache-dir.js')
 const isPackageBin = require('./util/is-package-bin.js')
 const removeTrailingSlashes = require('./util/trailing-slashes.js')
-const getContents = require('@npmcli/installed-package-contents')
-const PackageJson = require('@npmcli/package-json')
-const { Minipass } = require('minipass')
-const cacheDir = require('./util/cache-dir.js')
-const _ = require('./util/protected.js')
 
 // Pacote is only concerned with the package.json contents
 const packageJsonPrepare = (p) => PackageJson.prepare(p).then(pkg => pkg.content)
@@ -337,12 +337,12 @@ class FetcherBase {
 
   #empty (path) {
     return getContents({ path, depth: 1 }).then(contents => Promise.all(
-      contents.map(entry => fs.rm(entry, { recursive: true, force: true }))))
+      contents.map(entry => rm(entry, { recursive: true, force: true }))))
   }
 
   async #mkdir (dest) {
     await this.#empty(dest)
-    return await fs.mkdir(dest, { recursive: true })
+    return await mkdir(dest, { recursive: true })
   }
 
   // extraction is always the same.  the only difference is where
@@ -369,7 +369,7 @@ class FetcherBase {
   // don't use this.#mkdir because we don't want to rimraf anything
   async tarballFile (dest) {
     const dir = dirname(dest)
-    await fs.mkdir(dir, { recursive: true })
+    await mkdir(dir, { recursive: true })
     return this.#toFile(dest)
   }
 

lib/file.js

@@ -1,7 +1,7 @@
-const fsm = require('fs-minipass')
+const { resolve } = require('node:path')
+const { stat, chmod } = require('node:fs/promises')
 const cacache = require('cacache')
-const { resolve } = require('path')
-const { stat, chmod } = require('fs/promises')
+const fsm = require('fs-minipass')
 const Fetcher = require('./fetcher.js')
 const _ = require('./util/protected.js')
 

lib/git.js

@@ -1,16 +1,16 @@
-const Fetcher = require('./fetcher.js')
-const FileFetcher = require('./file.js')
-const RemoteFetcher = require('./remote.js')
-const DirFetcher = require('./dir.js')
+const cacache = require('cacache')
 const git = require('@npmcli/git')
-const pickManifest = require('npm-pick-manifest')
 const npa = require('npm-package-arg')
+const pickManifest = require('npm-pick-manifest')
 const { Minipass } = require('minipass')
-const cacache = require('cacache')
 const { log } = require('proc-log')
-const npm = require('./util/npm.js')
-const addGitSha = require('./util/add-git-sha.js')
+const DirFetcher = require('./dir.js')
+const Fetcher = require('./fetcher.js')
+const FileFetcher = require('./file.js')
+const RemoteFetcher = require('./remote.js')
 const _ = require('./util/protected.js')
+const addGitSha = require('./util/add-git-sha.js')
+const npm = require('./util/npm.js')
 
 const hashre = /^[a-f0-9]{40}$/
 

lib/registry.js

@@ -1,14 +1,14 @@
-const Fetcher = require('./fetcher.js')
-const RemoteFetcher = require('./remote.js')
-const pacoteVersion = require('../package.json').version
-const removeTrailingSlashes = require('./util/trailing-slashes.js')
+const crypto = require('node:crypto')
 const PackageJson = require('@npmcli/package-json')
 const pickManifest = require('npm-pick-manifest')
 const ssri = require('ssri')
-const crypto = require('crypto')
 const npa = require('npm-package-arg')
 const sigstore = require('sigstore')
 const fetch = require('npm-registry-fetch')
+const Fetcher = require('./fetcher.js')
+const RemoteFetcher = require('./remote.js')
+const pacoteVersion = require('../package.json').version
+const removeTrailingSlashes = require('./util/trailing-slashes.js')
 const _ = require('./util/protected.js')
 
 // Corgis are cute. 🐕🐶

lib/remote.js

@@ -1,9 +1,9 @@
-const Fetcher = require('./fetcher.js')
-const FileFetcher = require('./file.js')
-const pacoteVersion = require('../package.json').version
 const fetch = require('npm-registry-fetch')
 const { Minipass } = require('minipass')
+const Fetcher = require('./fetcher.js')
+const FileFetcher = require('./file.js')
 const _ = require('./util/protected.js')
+const pacoteVersion = require('../package.json').version
 
 class RemoteFetcher extends Fetcher {
   constructor (spec, opts) {

lib/util/cache-dir.js

@@ -1,10 +1,10 @@
-const os = require('os')
-const { resolve } = require('path')
+const { resolve } = require('node:path')
+const { tmpdir, homedir } = require('node:os')
 
 module.exports = (fakePlatform = false) => {
-  const temp = os.tmpdir()
+  const temp = tmpdir()
   const uidOrPid = process.getuid ? process.getuid() : process.pid
-  const home = os.homedir() || resolve(temp, 'npm-' + uidOrPid)
+  const home = homedir() || resolve(temp, 'npm-' + uidOrPid)
   const platform = fakePlatform || process.platform
   const cacheExtra = platform === 'win32' ? 'npm-cache' : '.npm'
   const cacheRoot = (platform === 'win32' && process.env.LOCALAPPDATA) || home

lib/util/protected.js

@@ -1,11 +1,5 @@
-const readPackageJson = Symbol.for('package.Fetcher._readPackageJson')
-const prepareDir = Symbol('_prepareDir')
-const tarballFromResolved = Symbol.for('pacote.Fetcher._tarballFromResolved')
-const cacheFetches = Symbol.for('pacote.Fetcher._cacheFetches')
-
 module.exports = {
-  readPackageJson,
-  prepareDir,
-  tarballFromResolved,
-  cacheFetches,
+  cacheFetches: Symbol.for('pacote.Fetcher._cacheFetches'),
+  readPackageJson: Symbol.for('package.Fetcher._readPackageJson'),
+  tarballFromResolved: Symbol.for('pacote.Fetcher._tarballFromResolved'),
 }

test/bin.js

@@ -1,15 +1,15 @@
+const t = require('tap')
+const { spawn } = require('node:child_process')
+const { Minipass } = require('minipass')
 const pkg = require('../package.json')
 const bin = require.resolve(`../${pkg.bin.pacote}`)
 const { main, run, parseArg, parse } = require(bin)
-const { spawn } = require('child_process')
-const t = require('tap')
-const pacote = require('../')
-const { Minipass } = require('minipass')
 
 t.cleanSnapshot = str =>
   str.split(pkg.version).join('{VERSION}')
     .split(process.env.HOME).join('{HOME}')
 
+const pacote = require('../')
 pacote.resolve = (spec, conf) =>
   spec === 'fail' ? Promise.reject(new Error('fail'))
   : spec === 'string' ? Promise.resolve('just a string')

test/dir.js

@@ -2,8 +2,8 @@ const runScript = require('@npmcli/run-script')
 const RUNS = []
 const t = require('tap')
 const Arborist = require('@npmcli/arborist')
-const fs = require('fs')
-const { relative, resolve, basename } = require('path')
+const fs = require('node:fs')
+const { relative, resolve, basename } = require('node:path')
 
 const loadActual = async (path) => {
   const arb = new Arborist({ path })

test/fetcher.js

@@ -1,13 +1,14 @@
-const fs = require('fs')
-const { relative, resolve, basename } = require('path')
 const t = require('tap')
-const Fetcher = require('../lib/fetcher.js')
-const abbrevMani = require('./fixtures/abbrev-manifest-min.json')
+const fs = require('node:fs')
+const { relative, resolve, basename } = require('node:path')
 const cacache = require('cacache')
 const { Minipass } = require('minipass')
+const npa = require('npm-package-arg')
+// FUN FACT if we require FileFetcher first the code can't run cause of a circular dependency
+const Fetcher = require('../lib/fetcher.js')
 // we actually use a file fetcher for this, because we need implementations
 const FileFetcher = require('../lib/file.js')
-const npa = require('npm-package-arg')
+const abbrevMani = require('./fixtures/abbrev-manifest-min.json')
 
 const byDigest = cacache.get.stream.byDigest
 

test/file.js

@@ -1,7 +1,7 @@
-const FileFetcher = require('../lib/file.js')
 const t = require('tap')
-const { relative, resolve, basename } = require('path')
-const fs = require('fs')
+const { relative, resolve, basename } = require('node:path')
+const fs = require('node:fs')
+const FileFetcher = require('../lib/file.js')
 
 t.cleanSnapshot = str => str.split(process.cwd()).join('${CWD}')
 

test/git.js

@@ -1,18 +1,17 @@
-const GitFetcher = require('../lib/git.js')
 const t = require('tap')
-const fs = require('fs')
-const http = require('http')
-const { dirname, basename, resolve } = require('path')
+const fs = require('node:fs')
+const http = require('node:http')
+const { dirname, basename, resolve } = require('node:path')
+const { mkdir } = require('node:fs/promises')
+const { rmSync } = require('node:fs')
+const { spawn } = require('node:child_process')
+const Arborist = require('@npmcli/arborist')
 const HostedGit = require('hosted-git-info')
 const npa = require('npm-package-arg')
-const Arborist = require('@npmcli/arborist')
 const spawnGit = require('@npmcli/git').spawn
-const { spawn } = require('child_process')
-const spawnNpm = require('../lib/util/npm.js')
-const { mkdir } = require('fs/promises')
-const { rmSync } = require('fs')
-
 const tar = require('tar')
+const spawnNpm = require('../lib/util/npm.js')
+const GitFetcher = require('../lib/git.js')
 
 // set this up first, so we can use 127.0.0.1 as our "hosted git" service
 const httpPort = 18000 + (+process.env.TAP_CHILD_ID || 0)

test/index.js

@@ -1,12 +1,12 @@
 const t = require('tap')
-const pacote = require('../lib/index.js')
-const fs = require('fs')
+const fs = require('node:fs')
+const { resolve, relative } = require('node:path')
+const DirFetcher = require('../lib/dir.js')
+const FileFetcher = require('../lib/file.js')
 const GitFetcher = require('../lib/git.js')
 const RegistryFetcher = require('../lib/registry.js')
-const FileFetcher = require('../lib/file.js')
-const DirFetcher = require('../lib/dir.js')
 const RemoteFetcher = require('../lib/remote.js')
-const { resolve, relative } = require('path')
+const pacote = require('../lib/index.js')
 
 const abbrev = resolve(__dirname, 'fixtures/abbrev-1.1.1.tgz')
 const abbrevspec = `file:${relative(process.cwd(), abbrev)}`

test/registry.js

@@ -1,6 +1,6 @@
 const t = require('tap')
-const path = require('path')
-const fs = require('fs')
+const path = require('node:path')
+const fs = require('node:fs')
 const mr = require('npm-registry-mock')
 const tnock = require('./fixtures/tnock')
 // Stub out sigstore verification for testing to avoid needing to refresh the tuf cache
@@ -1236,7 +1236,7 @@ t.test('corgi packument is not cached as full packument', async t => {
     cache,
     fullMetadata: true,
   })
-  t.notEqual(await f.packument(), packument, 'did not get cached packument')
+  t.not(await f.packument(), packument, 'did not get cached packument')
   t.ok(packumentCache.has(`full:${packumentUrl}`), 'full packument is also now cached')
 })
 

test/remote.js

@@ -1,9 +1,9 @@
-const RemoteFetcher = require('../lib/remote.js')
-const http = require('http')
-const ssri = require('ssri')
 const t = require('tap')
-const { resolve } = require('path')
-const fs = require('fs')
+const fs = require('node:fs')
+const http = require('node:http')
+const { resolve } = require('node:path')
+const ssri = require('ssri')
+const RemoteFetcher = require('../lib/remote.js')
 
 const me = t.testdir()
 const cache = resolve(me, 'cache')

test/util/add-git-sha.js

@@ -1,6 +1,6 @@
 const t = require('tap')
-const addGitSha = require('../../lib/util/add-git-sha.js')
 const npa = require('npm-package-arg')
+const addGitSha = require('../../lib/util/add-git-sha.js')
 
 const cases = [
   // unknown host

test/util/cache-dir.js

@@ -1,17 +1,21 @@
 const t = require('tap')
-const os = require('os')
-
-os.tmpdir = () => '/tmp'
-os.homedir = () => '/home/isaacs'
-const path = require('path')
-path.resolve = path.posix.resolve
+const path = require('node:path')
 process.getuid = () => 69420
 process.env.LOCALAPPDATA = ''
 const isWindows = process.platform === 'win32'
 const posix = isWindows ? 'posix' : null
 const windows = isWindows ? null : 'win32'
 
-const cacheDir = require('../../lib/util/cache-dir.js')
+let homedir = '/home/isaacs'
+const cacheDir = t.mock('../../lib/util/cache-dir.js', {
+  'node:path': {
+    resolve: path.posix.resolve,
+  },
+  'node:os': {
+    tmpdir: () => '/tmp',
+    homedir: () => homedir,
+  },
+})
 
 // call it once just to cover the default arg setting
 // the tests all specify something, so they work predictably
@@ -23,7 +27,7 @@ t.equal(cacheDir(windows).cacache, '/home/isaacs/npm-cache/_cacache')
 t.equal(cacheDir(posix).tufcache, '/home/isaacs/.npm/_tuf')
 t.equal(cacheDir(windows).tufcache, '/home/isaacs/npm-cache/_tuf')
 
-os.homedir = () => null
+homedir = null
 t.equal(cacheDir(posix).cacache, '/tmp/npm-69420/.npm/_cacache')
 t.equal(cacheDir(windows).cacache, '/tmp/npm-69420/npm-cache/_cacache')
 t.equal(cacheDir(posix).tufcache, '/tmp/npm-69420/.npm/_tuf')

test/util/is-package-bin.js

@@ -1,5 +1,5 @@
-const isPackageBin = require('../../lib/util/is-package-bin.js')
 const t = require('tap')
+const isPackageBin = require('../../lib/util/is-package-bin.js')
 
 t.ok(isPackageBin({ bin: 'foo' }, 'package/foo'), 'finds string')
 t.ok(isPackageBin({ bin: { bar: 'foo' } }, 'package/foo'), 'finds in obj')