fix: correctly handle workspaces and workspace roots

this backports the changes from #102

lib/index.js

@@ -2,8 +2,8 @@
 
 const Arborist = require('@npmcli/arborist')
 const { Walker: IgnoreWalker } = require('ignore-walk')
-const { lstatSync: lstat } = require('fs')
-const { basename, extname, join, relative, resolve } = require('path')
+const { lstatSync: lstat, readFileSync: readFile } = require('fs')
+const { basename, dirname, extname, join, relative, resolve, sep } = require('path')
 
 // symbols used to represent synthetic rule sets
 const defaultRules = Symbol('npm-packlist.rules.default')
@@ -48,6 +48,35 @@ const strictDefaults = [
   '/.git',
 ]
 
+const normalizePath = (path) => path.split('\\').join('/')
+
+const readOutOfTreeIgnoreFiles = (root, rel, result = []) => {
+  for (const file of ['.gitignore', '.npmignore']) {
+    try {
+      const ignoreContent = readFile(join(root, file), { encoding: 'utf8' })
+      result.push(ignoreContent)
+    } catch (err) {
+      // we ignore ENOENT errors completely because we don't care if the file doesn't exist
+      // but we throw everything else because failing to read a file that does exist is
+      // something that the user likely wants to know about
+      // istanbul ignore next -- we do not need to test a thrown error
+      if (err.code !== 'ENOENT') {
+        throw err
+      }
+    }
+  }
+
+  if (!rel) {
+    return result
+  }
+
+  const firstRel = rel.split(sep)[0]
+  const newRoot = join(root, firstRel)
+  const newRel = relative(newRoot, join(root, rel))
+
+  return readOutOfTreeIgnoreFiles(newRoot, newRel, result)
+}
+
 class PackWalker extends IgnoreWalker {
   constructor (opts) {
     const options = {
@@ -75,8 +104,30 @@ class PackWalker extends IgnoreWalker {
     this.tree = options.tree // no default, we'll load the tree later if we need to
     this.requiredFiles = options.requiredFiles || []
 
+    const additionalDefaults = []
+    if (options.prefix && options.workspaces) {
+      const path = normalizePath(options.path)
+      const prefix = normalizePath(options.prefix)
+      const workspaces = options.workspaces.map((ws) => normalizePath(ws))
+
+      // istanbul ignore else - this does nothing unless we need it to
+      if (path !== prefix && workspaces.includes(path)) {
+        // if path and prefix are not the same directory, and workspaces has path in it
+        // then we know path is a workspace directory. in order to not drop ignore rules
+        // from directories between the workspaces root (prefix) and the workspace itself
+        // (path) we need to find and read those now
+        const relpath = relative(options.prefix, dirname(options.path))
+        additionalDefaults.push(...readOutOfTreeIgnoreFiles(options.prefix, relpath))
+      } else if (path === prefix) {
+        // on the other hand, if the path and prefix are the same, then we ignore workspaces
+        // so that we don't pack a workspace as part of the root project. append them as
+        // normalized relative paths from the root
+        additionalDefaults.push(...workspaces.map((w) => normalizePath(relative(options.path, w))))
+      }
+    }
+
     // go ahead and inject the default rules now
-    this.injectRules(defaultRules, defaults)
+    this.injectRules(defaultRules, [...defaults, ...additionalDefaults])
 
     if (!this.isPackage) {
       // if this instance is not a package, then place some strict default rules, and append

test/workspace.js

@@ -45,7 +45,8 @@ t.test('respects workspace root ignore files', async (t) => {
     'package.json',
   ])
 
-  // here we leave off workspaces to satisfy coverage
+  // leave off workspaces to prove that when prefix and root differ there is no change
+  // in behavior without also specifying workspaces
   const secondFiles = await packlist({
     path: workspacePath,
     prefix: root,
@@ -87,7 +88,7 @@ t.test('packing a workspace root does not include children', async (t) => {
   })
 
   const workspacePath = path.join(root, 'workspaces', 'foo')
-  // this simulates what it looks like when a user does i.e. npm pack from a workspace root
+  // this simulates what it looks like when a user does `npm pack` from a workspace root
   const files = await packlist({
     path: root,
     prefix: root,
@@ -98,6 +99,7 @@ t.test('packing a workspace root does not include children', async (t) => {
     'package.json',
   ])
 
+  // prove if we leave off workspaces we do not omit them
   const secondFiles = await packlist({
     path: root,
     prefix: root,
@@ -109,67 +111,3 @@ t.test('packing a workspace root does not include children', async (t) => {
     'workspaces/foo/package.json',
   ])
 })
-
-t.test('.gitignore is discarded if .npmignore exists outside of tree', async (t) => {
-  const root = t.testdir({
-    'package.json': JSON.stringify({
-      name: 'workspace-root',
-      version: '1.0.0',
-      main: 'root.js',
-      workspaces: ['./workspaces/foo'],
-    }),
-    'root.js': `console.log('hello')`,
-    '.gitignore': 'dont-ignore-me',
-    '.npmignore': 'only-ignore-me',
-    'dont-ignore-me': 'should not be ignored',
-    'only-ignore-me': 'should be ignored',
-    workspaces: {
-      '.gitignore': 'dont-ignore-me-either',
-      '.npmignore': 'ignore-me-also',
-      'dont-ignore-me': 'should not be ignored',
-      'dont-ignore-me-either': 'should not be ignored',
-      'only-ignore-me': 'should be ignored',
-      'ignore-me-also': 'should be ignored',
-      foo: {
-        'package.json': JSON.stringify({
-          name: 'workspace-child',
-          version: '1.0.0',
-          main: 'child.js',
-        }),
-        'child.js': `console.log('hello')`,
-        'dont-ignore-me': 'should not be ignored',
-        'dont-ignore-me-either': 'should not be ignored',
-        'only-ignore-me': 'should be ignored',
-        'ignore-me-also': 'should also be ignored',
-      },
-    },
-  })
-
-  const workspacePath = path.join(root, 'workspaces', 'foo')
-  // this simulates what it looks like when a user does i.e. npm pack -w ./workspaces/foo
-  const files = await packlist({
-    path: workspacePath,
-    prefix: root,
-    workspaces: [workspacePath],
-  })
-  t.same(files, [
-    'dont-ignore-me',
-    'dont-ignore-me-either',
-    'child.js',
-    'package.json',
-  ])
-
-  // here we leave off workspaces to satisfy coverage
-  const secondFiles = await packlist({
-    path: workspacePath,
-    prefix: root,
-  })
-  t.same(secondFiles, [
-    'dont-ignore-me',
-    'dont-ignore-me-either',
-    'ignore-me-also',
-    'only-ignore-me',
-    'child.js',
-    'package.json',
-  ])
-})