diff --git a/lib/index.js b/lib/index.js index 501dad8..f484dc8 100644 --- a/lib/index.js +++ b/lib/index.js @@ -5,9 +5,10 @@ const { minimatch } = require('minimatch') const rpj = require('read-package-json-fast') const { glob } = require('glob') -function appendNegatedPatterns (patterns) { - const results = [] - for (let pattern of patterns) { +function appendNegatedPatterns (allPatterns) { + const patterns = [] + const negatedPatterns = [] + for (let pattern of allPatterns) { const excl = pattern.match(/^!+/) if (excl) { pattern = pattern.slice(excl[0].length) @@ -18,10 +19,35 @@ function appendNegatedPatterns (patterns) { // an odd number of ! means a negated pattern. !!foo ==> foo const negate = excl && excl[0].length % 2 === 1 - results.push({ pattern, negate }) + if (negate) { + negatedPatterns.push(pattern) + } else { + // remove negated patterns that appeared before this pattern to avoid + // ignoring paths that were matched afterwards + // e.g: ['packages/**', '!packages/b/**', 'packages/b/a'] + // in the above list, the last pattern overrides the negated pattern + // right before it. In effect, the above list would become: + // ['packages/**', 'packages/b/a'] + // The order matters here which is why we must do it inside the loop + // as opposed to doing it all together at the end. + for (let i = 0; i < negatedPatterns.length; ++i) { + const negatedPattern = negatedPatterns[i] + if (minimatch(pattern, negatedPattern)) { + negatedPatterns.splice(i, 1) + } + } + patterns.push(pattern) + } } - return results + // use the negated patterns to eagerly remove all the patterns that + // can be removed to avoid unnecessary crawling + for (const negated of negatedPatterns) { + for (const pattern of minimatch.match(patterns, negated)) { + patterns.splice(patterns.indexOf(pattern), 1) + } + } + return { patterns, negatedPatterns } } function getPatterns (workspaces) { @@ -77,11 +103,11 @@ async function mapWorkspaces (opts = {}) { } const { workspaces = [] } = opts.pkg - const patterns = getPatterns(workspaces) + const { patterns, negatedPatterns } = getPatterns(workspaces) const results = new Map() const seen = new Map() - if (!patterns.length) { + if (!patterns.length && !negatedPatterns.length) { return results } @@ -89,52 +115,54 @@ async function mapWorkspaces (opts = {}) { ...opts, ignore: [ ...opts.ignore || [], - ...['**/node_modules/**'], + '**/node_modules/**', + // just ignore the negated patterns to avoid unnecessary crawling + ...negatedPatterns, ], }) const getPackagePathname = pkgPathmame(opts) - for (const item of patterns) { - let matches = await glob(getGlobPattern(item.pattern), getGlobOpts()) - // preserves glob@8 behavior - matches = matches.sort((a, b) => a.localeCompare(b, 'en')) - - for (const match of matches) { - let pkg - const packageJsonPathname = getPackagePathname(match, 'package.json') - const packagePathname = path.dirname(packageJsonPathname) - - try { - pkg = await rpj(packageJsonPathname) - } catch (err) { - if (err.code === 'ENOENT') { - continue - } else { - throw err - } - } + let matches = await glob(patterns.map((p) => getGlobPattern(p)), getGlobOpts()) + // preserves glob@8 behavior + matches = matches.sort((a, b) => a.localeCompare(b, 'en')) + + // we must preserve the order of results according to the given list of + // workspace patterns + const orderedMatches = [] + for (const pattern of patterns) { + orderedMatches.push(...matches.filter((m) => { + return minimatch(m, pattern, { partial: true, windowsPathsNoEscape: true }) + })) + } - const name = getPackageName(pkg, packagePathname) + for (const match of orderedMatches) { + let pkg + const packageJsonPathname = getPackagePathname(match, 'package.json') - let seenPackagePathnames = seen.get(name) - if (!seenPackagePathnames) { - seenPackagePathnames = new Set() - seen.set(name, seenPackagePathnames) - } - if (item.negate) { - seenPackagePathnames.delete(packagePathname) + try { + pkg = await rpj(packageJsonPathname) + } catch (err) { + if (err.code === 'ENOENT') { + continue } else { - seenPackagePathnames.add(packagePathname) + throw err } } + + const packagePathname = path.dirname(packageJsonPathname) + const name = getPackageName(pkg, packagePathname) + + let seenPackagePathnames = seen.get(name) + if (!seenPackagePathnames) { + seenPackagePathnames = new Set() + seen.set(name, seenPackagePathnames) + } + seenPackagePathnames.add(packagePathname) } 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 { @@ -177,30 +205,25 @@ mapWorkspaces.virtual = function (opts = {}) { const { workspaces = [] } = packages[''] || {} // uses a pathname-keyed map in order to negate the exact items const results = new Map() - const patterns = getPatterns(workspaces) - if (!patterns.length) { + const { patterns, negatedPatterns } = getPatterns(workspaces) + if (!patterns.length && !negatedPatterns.length) { return results } - patterns.push({ pattern: '**/node_modules/**', negate: true }) - - const getPackagePathname = pkgPathmame(opts) + negatedPatterns.push('**/node_modules/**') - for (const packageKey of Object.keys(packages)) { - if (packageKey === '') { - continue + const packageKeys = Object.keys(packages) + for (const pattern of negatedPatterns) { + for (const packageKey of minimatch.match(packageKeys, pattern)) { + packageKeys.splice(packageKeys.indexOf(packageKey), 1) } + } - for (const item of patterns) { - if (minimatch(packageKey, item.pattern)) { - const packagePathname = getPackagePathname(packageKey) - const name = getPackageName(packages[packageKey], packagePathname) - - if (item.negate) { - results.delete(packagePathname) - } else { - results.set(packagePathname, name) - } - } + const getPackagePathname = pkgPathmame(opts) + for (const pattern of patterns) { + for (const packageKey of minimatch.match(packageKeys, pattern)) { + const packagePathname = getPackagePathname(packageKey) + const name = getPackageName(packages[packageKey], packagePathname) + results.set(packagePathname, name) } } diff --git a/tap-snapshots/test/test.js.test.cjs b/tap-snapshots/test/test.js.test.cjs index 6d1caeb..21446a1 100644 --- a/tap-snapshots/test/test.js.test.cjs +++ b/tap-snapshots/test/test.js.test.cjs @@ -11,9 +11,9 @@ Map { } ` -exports[`test/test.js TAP double negate patterns > should include doubly-negated items into resulting map 1`] = ` +exports[`test/test.js TAP double negated patterns > should include doubly-negated items into resulting map 1`] = ` Map { - "a" => "{CWD}/test/tap-testdir-test-double-negate-patterns/packages/a", + "a" => "{CWD}/test/tap-testdir-test-double-negated-patterns/packages/a", } ` @@ -78,13 +78,13 @@ package 'b' has conflicts in the following paths: } ` -exports[`test/test.js TAP multiple negate patterns > should not include any negated pattern 1`] = ` +exports[`test/test.js TAP multiple negated patterns > should not include any negated pattern 1`] = ` Map {} ` -exports[`test/test.js TAP negate pattern > should not include negated patterns 1`] = ` +exports[`test/test.js TAP negated pattern > should not include negated patterns 1`] = ` Map { - "a" => "{CWD}/test/tap-testdir-test-negate-pattern/packages/a", + "a" => "{CWD}/test/tap-testdir-test-negated-pattern/packages/a", } ` @@ -114,6 +114,12 @@ Map { } ` +exports[`test/test.js TAP overlapping negated pattern > should not include negated patterns 1`] = ` +Map { + "a" => "{CWD}/test/tap-testdir-test-overlapping-negated-pattern/packages/a", +} +` + exports[`test/test.js TAP root declared within workspaces > should allow the root package to be declared within workspaces 1`] = ` Map { "a" => "{CWD}/test/tap-testdir-test-root-declared-within-workspaces/packages/a", @@ -135,7 +141,7 @@ Map { } ` -exports[`test/test.js TAP triple negate patterns > should exclude thrice-negated items from resulting map 1`] = ` +exports[`test/test.js TAP triple negated patterns > should exclude thrice-negated items from resulting map 1`] = ` Map {} ` diff --git a/test/test.js b/test/test.js index 0d832c9..77161f5 100644 --- a/test/test.js +++ b/test/test.js @@ -649,7 +649,7 @@ test('ignore option', t => { ) }) -test('negate pattern', t => { +test('negated pattern', t => { const cwd = t.testdir({ foo: { bar: { @@ -692,7 +692,7 @@ test('negate pattern', t => { ) }) -test('multiple negate patterns', t => { +test('multiple negated patterns', t => { const cwd = t.testdir({ foo: { bar: { @@ -737,7 +737,51 @@ test('multiple negate patterns', t => { ) }) -test('double negate patterns', t => { +test('overlapping negated pattern', t => { + const cwd = t.testdir({ + foo: { + bar: { + baz: { + e: { + 'package.json': '{ "name": "e" }', + }, + }, + node_modules: { + b: { + 'package.json': '{ "name": "b" }', + }, + }, + }, + }, + packages: { + a: { + 'package.json': '{ "name": "a" }', + node_modules: { + c: { + 'package.json': '{ "name": "c" }', + }, + }, + }, + }, + }) + + return t.resolveMatchSnapshot( + mapWorkspaces({ + cwd, + pkg: { + workspaces: [ + 'packages/*', + 'foo/**', + '**/baz/**', + '!**/baz/**', + ], + }, + }), + 'should not include negated patterns' + ) +}) + +test('double negated patterns', t => { const cwd = t.testdir({ packages: { a: { @@ -759,7 +803,7 @@ test('double negate patterns', t => { ) }) -test('triple negate patterns', t => { +test('triple negated patterns', t => { const cwd = t.testdir({ packages: { a: {