-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat: migrate minimatch
to micromatch
#240
base: master
Are you sure you want to change the base?
Conversation
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@types/[email protected], npm/[email protected] |
61d606d
to
58fc0c3
Compare
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
88acf82
to
eb6cc43
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we only use makeRe
and isMatch
, IMHO we can use picomatch
(the library the powers micromatch
under the hood) directly!
Yeah, I started with |
// only run `isMatch` when necessary | ||
!(options.exceptions && isMatch(filename, options.exceptions)) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// inside create
const isMatch = options.exceptions
? picomatch(options.exceptions)
: () => false;
// before return
// now we hoist the `isMatch` function, prevent creating multiple instance across AST Nodes
), | ||
) | ||
ignoreGlobs && | ||
isMatch(node.parent.source.value, ignoreGlobs, { matchBase: true }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use picomatch to hoist isMatch
function outside of AST traverse as well.
I commented on your upstream issue WRT the behavior of leading Since you had the forethought to refactor out the glob calls, we should be able to pretty easily add normalization by modifying the functions provided in console.log(isMatch('baz', '[a-z]*'))
// true
console.log(isMatch('./baz', './[a-z]*'))
// false
console.log(isMatch('./baz', './[a-z]*', { format: (path) => path.replace(/^\.\//, '') }))
// true |
Thanks @Kenneth-Sills I'll give it a try! Sorry, but it makes console.log(isMatch('baz', './[a-z]*', { format: (path) => path.replace(/^\.\//, '') }))
// true
console.log(isMatch('./baz', './[a-z]*', { format: (path) => path.replace(/^\.\//, '') }))
// true My hacking workaround: import { isMatch as isMatch_ } from 'micromatch'
const normalizeBackslashes = (str: string) => str.replaceAll('\\', '/')
const defaultFormat = (path: string) => path.replace(/^\.\//, '')
const isMatchBase = (path: string, pattern: string, options?: Options) => {
path = normalizeBackslashes(path)
pattern = normalizeBackslashes(pattern)
if (path.startsWith('./') && !/^(\.\/|\*{1,2})/.test(pattern)) {
return false
}
return isMatch_(path, pattern, { format: defaultFormat, ...options })
}
export const isMatch = (
pathname: string,
patterns: string | string[],
options?: Options,
) => {
patterns = Array.isArray(patterns) ? patterns : [patterns]
return patterns.some(p => isMatchBase(pathname, p, options))
} |
eb6cc43
to
bc58783
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #240 +/- ##
==========================================
- Coverage 96.09% 96.07% -0.02%
==========================================
Files 106 107 +1
Lines 4783 4794 +11
Branches 1619 1645 +26
==========================================
+ Hits 4596 4606 +10
- Misses 187 188 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bc58783
to
58a0f45
Compare
58a0f45
to
b22751e
Compare
blocked by micromatch/micromatch#279