feat: remove dependency on arborist, require tree to be passed in

BREAKING CHANGE: the arborist tree must now be provided in the options and will not be generated for you. the npm-packlist bin has also been removed.
npm · Sep 26, 2022 · 123875a · 123875a
1 parent 33d3354
commit 123875a
Show file tree

Hide file tree

Showing 50 changed files with 240 additions and 94 deletions.
diff --git a/README.md b/README.md
@@ -6,20 +6,24 @@ These can be handed to [tar](http://npm.im/tar) like so to make an npm
 package tarball:
 
 ```js
+const Arborist = require('@npmcli/arborist')
 const packlist = require('npm-packlist')
 const tar = require('tar')
 const packageDir = '/path/to/package'
 const packageTarball = '/path/to/package.tgz'
 
-packlist({ path: packageDir })
-  .then(files => tar.create({
-    prefix: 'package/',
-    cwd: packageDir,
-    file: packageTarball,
-    gzip: true
-  }, files))
-  .then(_ => {
-    // tarball has been created, continue with your day
+const arborist = new Arborist({ path: packageDir })
+arborist.loadActual().then((tree) => {
+  packlist({ path: packageDir, tree })
+    .then(files => tar.create({
+      prefix: 'package/',
+      cwd: packageDir,
+      file: packageTarball,
+      gzip: true
+    }, files))
+    .then(_ => {
+      // tarball has been created, continue with your day
+    })
   })
 ```
 
@@ -97,7 +101,6 @@ Any specific file matched by an exact filename in the package.json `files` list
 
 ## API
 
-Same API as [ignore-walk](http://npm.im/ignore-walk), just hard-coded
-file list and rule sets.
+Same API as [ignore-walk](http://npm.im/ignore-walk), except providing a `tree` is required and there are hard-coded file list and rule sets.
 
-The `Walker` class will load an [arborist](https://github.com/npm/cli/tree/latest/workspaces/arborist) tree, and if any bundled dependencies are found will include them as well as their own dependencies in the resulting file set.
+The `Walker` class requires an [arborist](https://github.com/npm/cli/tree/latest/workspaces/arborist) tree, and if any bundled dependencies are found will include them as well as their own dependencies in the resulting file set.
diff --git a/lib/index.js b/lib/index.js
@@ -1,6 +1,5 @@
 'use strict'
 
-const Arborist = require('@npmcli/arborist')
 const { Walker: IgnoreWalker } = require('ignore-walk')
 const { lstatSync: lstat, readFileSync: readFile } = require('fs')
 const { basename, dirname, extname, join, relative, resolve, sep } = require('path')
@@ -104,7 +103,7 @@ class PackWalker extends IgnoreWalker {
     super(options)
     this.isPackage = options.isPackage
     this.seen = options.seen || new Set()
-    this.tree = options.tree // no default, we'll load the tree later if we need to
+    this.tree = options.tree
     this.requiredFiles = options.requiredFiles || []
 
     const additionalDefaults = []
@@ -202,22 +201,6 @@ class PackWalker extends IgnoreWalker {
     return super.stat(opts, callback)
   }
 
-  // overridden method: we need to load the arborist tree before we actually start running
-  start () {
-    if (this.isPackage && !this.tree) {
-      const arborist = new Arborist({ path: this.path })
-      // loading the tree is async while the start function depends on being sync
-      // eslint-disable-next-line promise/catch-or-return, promise/always-return
-      arborist.loadActual().then((tree) => {
-        this.tree = tree
-        super.start()
-      })
-      return this
-    }
-
-    return super.start()
-  }
-
   // overridden method: this is called to create options for a child walker when we step
   // in to a normal child directory (this will never be a bundle). the default method here
   // copies the root's `ignoreFiles` value, but we don't want to respect package.json for

diff --git a/package.json b/package.json
@@ -7,7 +7,6 @@
   },
   "main": "lib/index.js",
   "dependencies": {
-    "@npmcli/arborist": "^5.0.4 || ^6.0.0 || ^6.0.0-pre.0",
     "ignore-walk": "^5.0.1"
   },
   "author": "GitHub Inc.",
@@ -17,6 +16,7 @@
     "lib/"
   ],
   "devDependencies": {
+    "@npmcli/arborist": "^5.0.4 || ^6.0.0 || ^6.0.0-pre.0",
     "@npmcli/eslint-config": "^3.0.1",
     "@npmcli/template-oss": "4.4.2",
     "mutate-fs": "^2.1.1",
@@ -45,10 +45,8 @@
     "nyc-arg": [
       "--exclude",
       "tap-snapshots/**"
-    ]
-  },
-  "bin": {
-    "npm-packlist": "bin/index.js"
+    ],
+    "files": ["test/*.js"]
   },
   "engines": {
     "node": "^14.17.0 || ^16.13.0 || >=18.0.0"

diff --git a/test/bin.js b/test/bin.js
@@ -6,7 +6,7 @@ const t = require('tap')
 const { spawnSync } = require('child_process')
 
 const nodePath = process.execPath
-const binPath = require.resolve('../bin/index.js')
+const binPath = require.resolve('./utils/bin.js')
 
 const cwd = t.testdir({
   'package.json': JSON.stringify({

diff --git a/test/bundle-missing-dep.js b/test/bundle-missing-dep.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const Arborist = require('@npmcli/arborist')
 const t = require('tap')
 const packlist = require('../')
 
@@ -24,8 +25,10 @@ t.test('skips bundling deps with missing edges', async (t) => {
       },
     },
   })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
 
-  const files = await packlist({ path: pkg })
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     'index.js',
     'package.json',

diff --git a/test/bundled-cycle.js b/test/bundled-cycle.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const Arborist = require('@npmcli/arborist')
 const t = require('tap')
 const packlist = require('../')
 
@@ -41,7 +42,9 @@ t.test('correctly bundles cyclic deps', async (t) => {
     },
   })
 
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     'index.js',
     'node_modules/a/index.js',

diff --git a/test/bundled-file-in-workspace.js b/test/bundled-file-in-workspace.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const Arborist = require('@npmcli/arborist')
 const t = require('tap')
 const packlist = require('../')
 
@@ -29,7 +30,9 @@ t.test('correctly filters files from workspace subdirectory', async (t) => {
     },
   })
 
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     'index.js',
     'package.json',
@@ -76,7 +79,9 @@ t.test('does not filter based on package.json if subdirectory is not a workspace
     },
   })
 
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     'index.js',
     'package.json',

diff --git a/test/bundled-files.js b/test/bundled-files.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const Arborist = require('@npmcli/arborist')
 const t = require('tap')
 const packlist = require('../')
 
@@ -36,7 +37,9 @@ t.test('includes bundled dependency using bundleDependencies', async (t) => {
     },
   })
 
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     'elf.js',
     'node_modules/history/index.js',
@@ -73,7 +76,9 @@ t.test('includes bundled dependency using bundledDependencies', async (t) => {
     },
   })
 
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     'elf.js',
     'node_modules/history/index.js',

diff --git a/test/bundled-scoped-symlink.js b/test/bundled-scoped-symlink.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const Arborist = require('@npmcli/arborist')
 const t = require('tap')
 const packlist = require('..')
 
@@ -51,7 +52,9 @@ const pkg = t.testdir({
 }) + '/pkg'
 
 t.test('includes bundled dependency', async (t) => {
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     'elf.js',
     'node_modules/@npmwombat/history/index.js',

diff --git a/test/bundled-scoped.js b/test/bundled-scoped.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const Arborist = require('@npmcli/arborist')
 const t = require('tap')
 const packlist = require('../')
 
@@ -37,7 +38,9 @@ const pkg = t.testdir({
 })
 
 t.test('includes bundled dependency', async (t) => {
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     'elf.js',
     'node_modules/@npmwombat/history/index.js',

diff --git a/test/bundled-symlink.js b/test/bundled-symlink.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const Arborist = require('@npmcli/arborist')
 const t = require('tap')
 const packlist = require('../')
 
@@ -49,7 +50,9 @@ const pkg = t.testdir({
 }) + '/pkg'
 
 t.test('includes bundled dependency', async (t) => {
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     'elf.js',
     'node_modules/history/index.js',

diff --git a/test/bundled-workspace.js b/test/bundled-workspace.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const Arborist = require('@npmcli/arborist')
 const t = require('tap')
 const packlist = require('../')
 
@@ -42,7 +43,9 @@ t.test('packs workspace dependencies correctly', async (t) => {
     },
   })
 
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     'index.js',
     'node_modules/bar/index.js',

diff --git a/test/bundled.js b/test/bundled.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const Arborist = require('@npmcli/arborist')
 const t = require('tap')
 const packlist = require('../')
 
@@ -35,7 +36,9 @@ t.test('includes bundled dependency using bundleDependencies', async (t) => {
     },
   })
 
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     'elf.js',
     'node_modules/history/index.js',
@@ -71,7 +74,9 @@ t.test('includes bundled dependency using bundledDependencies', async (t) => {
     },
   })
 
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     'elf.js',
     'node_modules/history/index.js',

diff --git a/test/callbacks.js b/test/callbacks.js
@@ -1,5 +1,6 @@
 'use strict'
 
+const Arborist = require('@npmcli/arborist')
 const t = require('tap')
 const packlist = require('../')
 
@@ -9,15 +10,20 @@ const pkg = t.testdir({
   }),
 })
 
-t.test('export supports callbacks', (t) => {
-  packlist({ path: pkg }, (err, files) => {
-    if (err) {
-      throw err
-    }
+t.test('export supports callbacks', async (t) => {
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
 
-    t.same(files, [
-      'package.json',
-    ])
-    t.end()
+  return new Promise((resolve, reject) => {
+    packlist({ path: pkg, tree }, (err, files) => {
+      if (err) {
+        reject(err)
+      }
+
+      t.same(files, [
+        'package.json',
+      ])
+      resolve(files)
+    })
   })
 })
diff --git a/test/cannot-exclude-package-json.js b/test/cannot-exclude-package-json.js
@@ -1,6 +1,7 @@
 // cannot exclude package.json in the root
 'use strict'
 
+const Arborist = require('@npmcli/arborist')
 const t = require('tap')
 const packlist = require('../')
 
@@ -12,7 +13,9 @@ const pkg = t.testdir({
 })
 
 t.test('try to exclude package.json but cannot', async (t) => {
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     '.npmignore',
     'package.json',

diff --git a/test/cannot-exclude-readme.js b/test/cannot-exclude-readme.js
@@ -1,6 +1,7 @@
 // cannot exclude readme.md in the root
 'use strict'
 
+const Arborist = require('@npmcli/arborist')
 const t = require('tap')
 const packlist = require('../')
 
@@ -18,7 +19,9 @@ const pkg = t.testdir({
 })
 
 t.test('try to exclude package.json but cannot', async (t) => {
-  const files = await packlist({ path: pkg })
+  const arborist = new Arborist({ path: pkg })
+  const tree = await arborist.loadActual()
+  const files = await packlist({ path: pkg, tree })
   t.same(files, [
     '.npmignore',
     'package.json',