Skip to content

Conversation

@Ugzuzg
Copy link

@Ugzuzg Ugzuzg commented Jul 28, 2025

Fixes #657

In a pnpm monorepo, I have the following patterns to upload with upload-artifact:

apps
packages
!**/node_modules

However, because a deep apps/app-a/node_modules/abc is a partial match, the globber goes through files in a directory that will be filtered out later anyway.

This reduces the time to find all matches from 1m 8s to 250ms with identical result arrays.

@Copilot Copilot AI review requested due to automatic review settings July 28, 2025 15:19
@Ugzuzg Ugzuzg requested a review from a team as a code owner July 28, 2025 15:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves glob performance by modifying the partialMatch function to respect negate patterns when determining whether to descend into directories. The change prevents the globber from traversing deep directory structures that would be filtered out by negate patterns anyway, significantly reducing glob operation time from over 1 minute to 250ms in the described scenario.

  • Modified the partialMatch function to check for negated patterns that match the current path
  • Updated corresponding test to reflect the new behavior where negated paths are not traversed

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/glob/src/internal-pattern-helper.ts Enhanced partialMatch function to avoid descending into directories matched by negate patterns
packages/glob/tests/internal-pattern-helper.test.ts Updated test case to verify the new negate pattern behavior

Comment on lines +80 to +83
return (
patterns.some(x => !x.negate && x.partialMatch(itemPath)) &&
!patterns.some(x => x.negate && x.match(itemPath))
)
Copy link

Copilot AI Jul 28, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider short-circuiting the evaluation by checking negate patterns first, since if any negate pattern matches, the result will be false regardless of positive patterns. This could be written as: if (patterns.some(x => x.negate && x.match(itemPath))) return false; return patterns.some(x => !x.negate && x.partialMatch(itemPath));

Suggested change
return (
patterns.some(x => !x.negate && x.partialMatch(itemPath)) &&
!patterns.some(x => x.negate && x.match(itemPath))
)
// Check negate patterns first for short-circuiting
if (patterns.some(x => x.negate && x.match(itemPath))) {
return false;
}
// Check positive patterns
return patterns.some(x => !x.negate && x.partialMatch(itemPath));

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fail-fast on children of negated glob directory matches like Git does

1 participant