Fix files.matches not working when it's a directory match #15
Conversation
src/utils/matches.ts
Outdated
| import micromatch from "micromatch" | ||
|
|
||
| export default function matches(paths: string[], patterns: string[]): string[] { | ||
| const isBasename = patterns.every((pattern) => path.basename(pattern) === pattern) |
There was a problem hiding this comment.
nit: Could we add a short comment about this workaround so it has a bit of context?
There was a problem hiding this comment.
Also, IIUC this might have unexpected behavior if some patterns are fully qualified and some are not -- am I reading this right?
There was a problem hiding this comment.
commented! and yeah hmmmm. I definitely chose the easy way out here. Most of the time everything only has one pattern. Maybe I should check and throw an error / warning if they're mixed?
There was a problem hiding this comment.
Yeah, that sounds good! (Seems like a good balance between a quick fix and hitting potentially difficult to debug unexpected behavior later.)
There was a problem hiding this comment.
Done. Looking a little clumsy, but I think much better that than having it silently / confusingly fail.
src/utils/matches.ts
Outdated
| for (const pattern of patterns) { | ||
| const isBasename = path.basename(pattern) === pattern | ||
| hasBasename ||= isBasename | ||
| hasNested ||= !isBasename | ||
| } |
There was a problem hiding this comment.
Should this loop just be a .find() so that it short-circuits?
There was a problem hiding this comment.
I think I'd need to do two finds, to check for both nested and basename. And if we're generally expecting only a couple (in most cases one) pattern, this could be slightly better. wdyt?
There was a problem hiding this comment.
Probably not a huge deal, doubt you're gonna have that many patterns.
There was a problem hiding this comment.
Makes sense, switched to find for clarity.
There was a problem hiding this comment.
@jamiebuilds hi hi! Could you help me figure out how to get this bugfix published? 🙏
|
Want to check if there are any further comments here! Otherwise I'd like to aim to merge / bump version tomorrow to get endanger working again. |
files.matchesno longer works when the pattern issome_directory/**/*, when thebasenameoption is true. Patterns with*.jsstill work fine.This seems to be an issue with the underlying micromatch/picomatch implementation: micromatch/picomatch#89. My expectation is that when the pattern has a slash,
basenameshould be ignored. No response to that issue though, so throwing a bandaid onto it to make our existing rules work again.