Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: faster workspace mapping #143

Merged
merged 5 commits into from
Apr 10, 2024
Merged

fix: faster workspace mapping #143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
23f192f
2x faster workspace mapping
thecodrr Dec 22, 2023
69c405c
update snapshots
thecodrr Dec 22, 2023
c570c9d
sort and fix the order
thecodrr Jan 4, 2024
ef9e3c3
preserve order of workspaces
thecodrr Jan 7, 2024
6397746
fixup: test coverage
wraithgar Apr 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 81 additions & 58 deletions lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -77,64 +103,66 @@ 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
}

const getGlobOpts = () => ({
...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 })
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not working correctly if workspaces contains leading ./ like: './packages/*'

}))
}

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 {
Expand Down Expand Up @@ -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)
}
}

Expand Down
18 changes: 12 additions & 6 deletions tap-snapshots/test/test.js.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
`

Expand Down Expand Up @@ -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",
}
`

Expand Down Expand Up @@ -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",
Expand All @@ -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 {}
`

Expand Down
52 changes: 48 additions & 4 deletions test/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ test('ignore option', t => {
)
})

test('negate pattern', t => {
test('negated pattern', t => {
const cwd = t.testdir({
foo: {
bar: {
Expand Down Expand Up @@ -692,7 +692,7 @@ test('negate pattern', t => {
)
})

test('multiple negate patterns', t => {
test('multiple negated patterns', t => {
const cwd = t.testdir({
foo: {
bar: {
Expand Down Expand Up @@ -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: {
Expand All @@ -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: {
Expand Down
Loading