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.

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

Ignore patterns are applied to the absolute path instead of relative to the working directory #441

Closed
func0der opened this issue May 18, 2024 · 4 comments · Fixed by #449 · May be fixed by #464
Closed

Ignore patterns are applied to the absolute path instead of relative to the working directory #441

func0der opened this issue May 18, 2024 · 4 comments · Fixed by #449 · May be fixed by #464
Milestone

Comments

@func0der
Copy link

Environment

  • OS Version: Linux
  • Node.js Version: 20.13.1

Actual behavior

[]

Expected behavior

['index.test.js']

Steps to reproduce

  1. Create a path ~/something/
  2. Put a file index.test.js in there
  3. Run the thing

Code sample

index.ts:

const fg = require('fast-glob');

const include = [ '**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}' ]
const globOptions = {
  absolute: true,
  ignore: [
    '**/something/**',
  ],
}

const foo = async () => {
        return await fg(include, globOptions);
}

foo().then(console.log);

The problem seems to be that the parameter absolute which is documented as an output control parameter (https://www.npmjs.com/package/fast-glob#output-control) is getting in the way of filtering.

I have not found the correct place in the code yet, but I figure that paths are RESOLVED first and then FILTERED, instead of FILTERED relatively to cwd first and then RESOLVED to their absolute counterparts.

I hope what I am getting to is clear. Maybe this is intentional behaviour, but i found no documented evidence of that anywhere.

@func0der
Copy link
Author

Failing test case:

In absolute.e2e.ts add:

		{
			pattern: 'file.md',
			options: {
				ignore: [path.posix.join('**', 'fixtures', '**')],
				cwd: 'fixtures',
				absolute: true,
			},
			expected: () => {
				return ['<root>/fixtures/file.md'];
			},
		}

It should fail.
I opened up a new PR for the failing test.

This never surfaced since the ABSOLUTE CWD was always added in all test cases in front of the ignore pattern.

@webpro
Copy link

webpro commented Jun 16, 2024

Facing the same issue. Just by looking at the code I'd guess the culprit is at utils.pattern.matchAny where the pattern (in patternsRe) now matches the fullpath.

Either:

  • patternsRe should only match against entryPath, even if absolute: true, but that would mean isSkippedByAbsoluteNegativePatterns could be removed altogether?
  • patternsRe should be filtered so it contains only absolute patterns before entering utils.pattern.matchAny, we could add an isAbsolutePattern() helper for this I guess

#isSkippedByAbsoluteNegativePatterns(entryPath: string, patternsRe: PatternRe[]): boolean {
if (!this.#settings.absolute) {
return false;
}
const fullpath = utils.path.makeAbsolute(this.#settings.cwd, entryPath);
return utils.pattern.matchAny(fullpath, patternsRe);
}

@mrmlnc What do you think? I'd be happy to try and submit a PR for this.

@webpro
Copy link

webpro commented Jun 16, 2024

PR: #445

@func0der
Copy link
Author

func0der commented Jul 4, 2024

Whoop. Nice. Let'S get that released.

Thanks for the hard work <3

@mrmlnc mrmlnc linked a pull request Nov 23, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants