feat: require arborist constructor to be passed in for preparing git …

…dirs (#204)

* feat: require arborist constructor to be passed in for preparing git dirs

This reverts the peer dependency change from d3517fd.

BREAKING CHANGE: a `@npmcli/arborist` constructor must be passed in if
no tree is provided and pacote is going to operate on git dependencies.

lib/dir.js

@@ -3,7 +3,6 @@ const FileFetcher = require('./file.js')
 const Minipass = require('minipass')
 const tarCreateOptions = require('./util/tar-create-options.js')
 const packlist = require('npm-packlist')
-const Arborist = require('@npmcli/arborist')
 const tar = require('tar')
 const _prepareDir = Symbol('_prepareDir')
 const { resolve } = require('path')
@@ -17,6 +16,9 @@ class DirFetcher extends Fetcher {
     super(spec, opts)
     // just the fully resolved filename
     this.resolved = this.spec.fetchSpec
+
+    this.tree = opts.tree || null
+    this.Arborist = opts.Arborist || null
   }
 
   // exposes tarCreateOptions as public API
@@ -60,6 +62,10 @@ class DirFetcher extends Fetcher {
   }
 
   [_tarballFromResolved] () {
+    if (!this.tree && !this.Arborist) {
+      throw new Error('DirFetcher requires either a tree or an Arborist constructor to pack')
+    }
+
     const stream = new Minipass()
     stream.resolved = this.resolved
     stream.integrity = this.integrity
@@ -71,7 +77,7 @@ class DirFetcher extends Fetcher {
     this[_prepareDir]()
       .then(async () => {
         if (!this.tree) {
-          const arb = new Arborist({ path: this.resolved })
+          const arb = new this.Arborist({ path: this.resolved })
           this.tree = await arb.loadActual()
         }
         return packlist(this.tree, { path: this.resolved, prefix, workspaces })

lib/fetcher.js

@@ -72,7 +72,6 @@ class FetcherBase {
 
     this.cache = opts.cache || cacheDir()
     this.resolved = opts.resolved || null
-    this.tree = opts.tree || null
 
     // default to caching/verifying with sha512, that's what we usually have
     // need to change this default, or start overriding it, when sha512

lib/git.js

@@ -61,6 +61,8 @@ class GitFetcher extends Fetcher {
     } else {
       this.resolvedSha = ''
     }
+
+    this.Arborist = opts.Arborist || null
   }
 
   // just exposed to make it easier to test all the combinations
@@ -206,8 +208,12 @@ class GitFetcher extends Fetcher {
     // check it out and then shell out to the DirFetcher tarball packer
     this[_clone](dir => this[_prepareDir](dir)
       .then(() => new Promise((res, rej) => {
+        if (!this.Arborist) {
+          throw new Error('GitFetcher requires an Arborist constructor to pack a tarball')
+        }
         const df = new DirFetcher(`file:${dir}`, {
           ...this.opts,
+          Arborist: this.Arborist,
           resolved: null,
           integrity: null,
         })

package.json

@@ -66,9 +66,6 @@
     "ssri": "^9.0.0",
     "tar": "^6.1.11"
   },
-  "peerDependencies": {
-    "@npmcli/arborist": "^6.0.0 || ^6.0.0-pre.0"
-  },
   "engines": {
     "node": "^14.17.0 || ^16.13.0 || >=18.0.0"
   },

test/dir.js

@@ -160,3 +160,8 @@ t.test('exposes tarCreateOptions method', async t => {
     'should return standard options'
   )
 })
+
+t.test('fails without a tree or constructor', async t => {
+  const f = new DirFetcher(abbrevspec, {})
+  t.rejects(() => f.extract(me + '/prepare'))
+})

test/fixtures/npm-mock.js

@@ -22,6 +22,7 @@ for (const [name, spec] of Object.entries(pkg.dependencies)) {
   pacote.extract(spec, process.cwd() + '/' + name, {
     npmBin: __filename,
     ...JSON.parse(pacoteOpts),
+    Arborist: require('@npmcli/arborist'),
   })
 }
 

test/git.js

@@ -78,7 +78,7 @@ const cycleB = resolve(me, 'cycle-b')
 
 const abbrevSpec = `file:${abbrev}`
 
-const opts = { cache }
+const opts = { cache, Arborist: require('@npmcli/arborist') }
 
 const spawnGit = require('@npmcli/git').spawn
 const { spawn } = require('child_process')
@@ -755,3 +755,9 @@ t.test('simple repo with only a prepack script', async t => {
     'should run prepack lifecycle script'
   )
 })
+
+t.test('fails without arborist constructor', async t => {
+  const ws = new GitFetcher(prepackRemote, { cache })
+  const extract = resolve(me, 'extract-prepack')
+  t.rejects(() => ws.extract(extract))
+})