fix: evaluate all patterns before throwing EDUPLICATEWORKSPACE (#32)

Author: louis-bompart

lib/index.js

@@ -117,28 +117,52 @@ async function mapWorkspaces (opts = {}) {
 
       const name = getPackageName(pkg, packagePathname)
 
+      let seenPackagePathnames = seen.get(name)
+      if (!seenPackagePathnames) {
+        seenPackagePathnames = new Set()
+        seen.set(name, seenPackagePathnames)
+      }
       if (item.negate) {
-        results.delete(packagePathname, name)
+        seenPackagePathnames.delete(packagePathname)
       } else {
-        if (seen.has(name) && seen.get(name) !== packagePathname) {
-          throw getError({
-            Type: Error,
-            message: [
-              'must not have multiple workspaces with the same name',
-              `package '${name}' has conflicts in the following paths:`,
-              '    ' + seen.get(name),
-              '    ' + packagePathname,
-            ].join('\n'),
-            code: 'EDUPLICATEWORKSPACE',
-          })
-        }
-
-        seen.set(name, packagePathname)
-        results.set(packagePathname, name)
+        seenPackagePathnames.add(packagePathname)
       }
     }
   }
-  return reverseResultMap(results)
+
+  const errorMessageArray = ['must not have multiple workspaces with the same name']
+  for (const [packageName, seenPackagePathnames] of seen) {
+    if (seenPackagePathnames.size === 0) {
+      continue
+    }
+    if (seenPackagePathnames.size > 1) {
+      addDuplicateErrorMessages(errorMessageArray, packageName, seenPackagePathnames)
+    } else {
+      results.set(packageName, seenPackagePathnames.values().next().value)
+    }
+  }
+
+  if (errorMessageArray.length > 1) {
+    throw getError({
+      Type: Error,
+      message: errorMessageArray.join('\n'),
+      code: 'EDUPLICATEWORKSPACE',
+    })
+  }
+
+  return results
+}
+
+function addDuplicateErrorMessages (messageArray, packageName, packagePathnames) {
+  messageArray.push(
+    `package '${packageName}' has conflicts in the following paths:`
+  )
+
+  for (const packagePathname of packagePathnames) {
+    messageArray.push(
+      '    ' + packagePathname
+    )
+  }
 }
 
 mapWorkspaces.virtual = function (opts = {}) {

tap-snapshots/test/test.js.test.cjs

@@ -41,6 +41,22 @@ Map {
 }
 `
 
+exports[`test/test.js TAP match duplicates then exclude one > should include the non-excluded item on returned Map 1`] = `
+Map {
+  "a" => "{CWD}/test/tap-testdir-test-match-duplicates-then-exclude-one/packages/a",
+}
+`
+
+exports[`test/test.js TAP matched then negated then match again > should include item on returned Map 1`] = `
+Map {
+  "a" => "{CWD}/test/tap-testdir-test-matched-then-negated-then-match-again/packages/b/a",
+}
+`
+
+exports[`test/test.js TAP matched then negated then match again with wildcards > should exclude item on returned Map 1`] = `
+Map {}
+`
+
 exports[`test/test.js TAP missing pkg info > should return an empty map 1`] = `
 Array [
   Map {},
@@ -49,6 +65,19 @@ Array [
 ]
 `
 
+exports[`test/test.js TAP multiple duplicated workspaces config > should throw an error listing all duplicates 1`] = `
+Error: must not have multiple workspaces with the same name
+package 'a' has conflicts in the following paths:
+    {CWD}/test/tap-testdir-test-multiple-duplicated-workspaces-config/packages/a
+    {CWD}/test/tap-testdir-test-multiple-duplicated-workspaces-config/packages/b
+    {CWD}/test/tap-testdir-test-multiple-duplicated-workspaces-config/packages/c
+package 'b' has conflicts in the following paths:
+    {CWD}/test/tap-testdir-test-multiple-duplicated-workspaces-config/packages/d
+    {CWD}/test/tap-testdir-test-multiple-duplicated-workspaces-config/packages/e {
+  "code": "EDUPLICATEWORKSPACE",
+}
+`
+
 exports[`test/test.js TAP multiple negate patterns > should not include any negated pattern 1`] = `
 Map {}
 `

test/test.js

@@ -7,7 +7,7 @@ tap.cleanSnapshot = str => {
   const cleanPath = path => path
     .replace(/\\+/g, '/') // normalize slashes
     .replace(/"\w:/g, '"') // gets rid of drive letter in snapshot
-    .replace(/^\w:/g, '') // gets rid of drive letter in cwd/paths
+    .replace(/^(\s*)\w:/gm, '$1') // gets rid of drive letter in cwd/paths
   const cwd = cleanPath(process.cwd())
   const pathname = cleanPath(str)
   return pathname.split(cwd).join('{CWD}')
@@ -229,6 +229,42 @@ test('duplicated workspaces glob pattern', t => {
   )
 })
 
+test('multiple duplicated workspaces config', t => {
+  const cwd = t.testdir({
+    packages: {
+      a: {
+        'package.json': '{ "name": "a" }',
+      },
+      b: {
+        'package.json': '{ "name": "a" }',
+      },
+      c: {
+        'package.json': '{ "name": "a" }',
+      },
+      d: {
+        'package.json': '{ "name": "b" }',
+      },
+      e: {
+        'package.json': '{ "name": "b" }',
+      },
+    },
+  })
+
+  return t.resolveMatchSnapshot(
+    mapWorkspaces({
+      cwd,
+      pkg: {
+        workspaces: {
+          packages: [
+            'packages/*',
+          ],
+        },
+      },
+    }).catch(error => Promise.resolve(error)),
+    'should throw an error listing all duplicates'
+  )
+})
+
 test('empty packages declaration', t => {
   const cwd = t.testdir({
     packages: {
@@ -793,3 +829,82 @@ test('backslashes are normalized', t => {
     'matches with backslashes'
   )
 })
+
+test('matched then negated then match again with wildcards', t => {
+  const cwd = t.testdir({
+    packages: {
+      b: {
+        a: {
+          'package.json': '{ "name": "a" }',
+        },
+      },
+    },
+  })
+
+  return t.resolveMatchSnapshot(
+    mapWorkspaces({
+      cwd,
+      pkg: {
+        workspaces: [
+          'packages/**',
+          '!packages/b/**',
+        ],
+      },
+    }),
+    'should exclude item on returned Map'
+  )
+})
+
+test('matched then negated then match again', t => {
+  const cwd = t.testdir({
+    packages: {
+      b: {
+        a: {
+          'package.json': '{ "name": "a" }',
+        },
+      },
+    },
+  })
+
+  return t.resolveMatchSnapshot(
+    mapWorkspaces({
+      cwd,
+      pkg: {
+        workspaces: [
+          'packages/**',
+          '!packages/b/**',
+          'packages/b/a',
+        ],
+      },
+    }),
+    'should include item on returned Map'
+  )
+})
+
+test('match duplicates then exclude one', t => {
+  const cwd = t.testdir({
+    packages: {
+      a: {
+        'package.json': '{ "name": "a" }',
+      },
+      b: {
+        a: {
+          'package.json': '{ "name": "a" }',
+        },
+      },
+    },
+  })
+
+  return t.resolveMatchSnapshot(
+    mapWorkspaces({
+      cwd,
+      pkg: {
+        workspaces: [
+          'packages/**',
+          '!packages/b/**',
+        ],
+      },
+    }),
+    'should include the non-excluded item on returned Map'
+  )
+})