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

Leading './' is not included in the glob regex #199

Open
valerybugakov opened this issue Apr 14, 2020 · 9 comments
Open

Leading './' is not included in the glob regex #199

valerybugakov opened this issue Apr 14, 2020 · 9 comments

Comments

@valerybugakov
Copy link

Minimum necessary steps to reproduce this issue:

const testGlob = '**/*.js';

console.log(micromatch.isMatch('./something/file.js', testGlob));
console.log(micromatch.isMatch('something/file.js', testGlob));
console.log(micromatch.makeRe(testGlob).test('./something/file.js'));
console.log(micromatch.makeRe(testGlob).test('something/file.js'));

What is happening that shouldn't be?

Output

true
true
false
true

What should be happening instead?

Expected

true
true
true
true

This issue is the exact duplicate of #110. It was resolved in nanomatch long ago. It works correctly using makeRe from nanomatch.

Should it be fixed in the same way in micromatch, probably in picomatch that is used under the hood?

@jonschlinkert
Copy link
Member

See the format option.

@valerybugakov
Copy link
Author

It solves the problem for isMatch.
I cannot figure out how to solve it for regex created via picomatch.makeRe(glob). Is it possible?

const format = str => str.replace(/^\.\//, '');
const regex = picomatch.makeRe('foo/*.js', { format });
console.log(regex.test('./foo/bar.js')); //=> false but expected true

@jonschlinkert
Copy link
Member

Is it possible?

Maybe try using the prepend option. However, IMHO the easiest and possibly the most reliable way to do this is to remove the ./ from paths before testing them. This is what I'd personally do if I was implementing a globbing library, since 1) it would most likely be faster than a more complicated regex, and 2) file paths do not typically have ./ before them. AFAIK that is only manually added when defining a require() path. I see devs add ./ before paths all the time and it doesn't make a lot of sense.

@valerybugakov
Copy link
Author

Agreed, but I have the same case as described in the issue linked above.

I am trying to convert the webpack require-context api into accepting a glob (it takes a regex). The strings it tests against that regex are always relative file paths that start with ./ and I cannot strip this prefix with their available api.

Is it safe to use nanomatch in that case? I see it hasn't been update for a while.

@valerybugakov
Copy link
Author

btw thanks for quick response 🙌

@jonschlinkert
Copy link
Member

Ah, I'm sorry for not reading more carefully and if my answer seemed to ignore those details. I didn't mean to be flippant. I see that this is more nuanced. Before I implement something new, let me think about this for a few, it seems like this came up before and I/we had a solution to it.

Is it safe to use nanomatch in that case? I see it hasn't been update for a while.

It's not as performant. I think we should try to make this work with picomatch. I thought the prepend option allowed you to add an arbitrary string to the beginning of the regex, which would allow you to add something like (\\.[\\\\/])?.

@valerybugakov
Copy link
Author

Got it, will try the prepend option for now.

@hydrosquall
Copy link

FWIW, minimatch has faced this problem for several years too, in case you want to collaborate on a solution.

isaacs/minimatch#30 (comment)

@jonschlinkert
Copy link
Member

@dawgwe1 you've been blocked

@micromatch micromatch deleted a comment Sep 25, 2021
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

No branches or pull requests

3 participants