Repository navigation
fs/path: expose internal-api minimatch/glob under path module. #40731
Description
Activity
Update to this: I'm in the process of some updates to minimatch and glob to:
- Bring it into the modern era (classes, promises, iterating/streaming results, etc.)
- Improve performance and avoid ReDOS threats by just parsing and evaluating without building up massive regexps.
- Support some edge cases that have always been a problem (especially: nested extglob patterns).
Once that's done, I'll be advocating for including it in core.
Reacted by Denis Malinochkin, kai zhu, Geoffrey Booth, Anton Kastritskii, Bruno Heridet, Bo Lingen and Daniel BayleyReacted by Denis Malinochkin and Owen BuckleyReacted by Michaël Zasso, Denis Malinochkin and Travis FischerReacted by Denis MalinochkinThere has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.
Reacted by Moshe AtlowThere has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.
For more information on how the project manages feature requests, please consult the feature request management document.
I think this would be a great addition, still! Can this issue be re-opened?
I am re-opening this issue since:
- it was raised again Request: mark test runner stable in Node 20.0.0 #46642 (comment)
- mini-match seems to have had a major release lately
@isaacs is this still true?
Once that's done, I'll be advocating for including it in core.
- just fyi, the original test-coverage-project that raised this issue been fixed (still would be a useful nodejs feature though)
- here's standalone, 300-line-gistfile of minimatch-alternative i've been using to batch-glob large number of files for test-coverage
Reacted by Moshe Atlow and Owen BuckleyYes, glob and minimatch have both been significantly updated lately. They're much faster, better designed, more faithfully implement bash pattern expansion semantics, and ship types.
I'm still not sure it makes sense to pull into node core though. Making glob fast did require a lot of moving parts. But the license allows it, so have at it if y'all want it in. I see no reason to recommend otherwise, and would be happy to help out if someone attempts this and runs into issues.
It might be desirable to make the glob stream interfaces node streams (or web streams) if so, as they're minipass streams to support sync globbing. It'll cost some perf to do that, tho. (Or we could talk about pulling in minipass, which wouldn't be a bad idea either, but that's of course a much bigger change.) Minimatch also exports its parser on minimatch.AST, which can be helpful if you want to use some parts but assemble it differently.
If you only need a subset of the bash globbing behavior, and especially if you don't need hot path performance or windows support, then a couple hundred line js module can definitely suffice. But I would not recommend pulling such a glob subset into core, unless the node team is interested in a never ending stream of bug reports for every edge case where it'll diverge from shell behavior. Having an incorrect or incomplete glob module in core is arguably worse than not having one at all.
Another completely different route to globbing in node core would be to explore doing it in C, by using the same actual code that expands patterns in bash or zsh. I'm not sure if their licenses would be compatible, of course, and bash globs are pretty slow on large folder trees, compared to node-glob or especially fastglob. But you can't get more consistent than "using the same code".
But anyway, yeah, if it was up to me, I'd say just let glob live in userland. It's fine there.
Reacted by Moshe Atlow, Khafra and Yahor Siarheyenka@kaizhu256 If you really want your tool to support globs faithfully, and remain "zero-dep", there's an easy solution.
npm install glob@latest --no-save mv node_modules vendor echo 'module.exports = require("../vendor/glob")' > lib/glob.js😜
Reacted by Debadree Chatterjee, Bruce Felt, Wojciech Maj and Ollie MonkBut anyway, yeah, if it was up to me, I'd say just let glob live in userland. It's fine there.
The issue mostly is that it's increasingly a common request needed by other Node.js features (like the test runner) that are seeing bigger and bigger adoption.
@benjamingr Then the thing to do, imo, is to bundle node-glob (if you want as correct/complete results as possible) or fast-glob (if you want as fast performance as possible), and call it a day. The difference is minor, node-glob is still very fast, and fast-glob is still very correct. Both are a much better fit for node than libglob can ever be.
Reacted by Moshe Atlow, Benjamin Gruenbaum and Bo LingenPersonally I agree and it's also likely better in terms of ease of understanding the code for most contributors, security and updates.
If Node wants to expose it as a built in API no one is stopping us from upstreaming changes making things like "throw Node's own errors" or "use primordials if node-glob is built with this or that flag" to node-glob or whatever we choose (in the error case Node can just wrap the errors like it does with other deps)
There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.
Is your feature request related to a problem? Please describe.
Please describe the problem you are trying to solve.
i have a zero-dependency coverage-reporting-tool and would like to add a cli-option to exclude code-coverage based on file-glob-patterns, e.g.:
node jslint.mjs v8_coverage_report \ --exclude=deps/*,test/* \ npm run testDescribe the solution you'd like
Please describe the desired behavior.
expose
./deps/npm/node_modules/minimatch/minimatch.jsunder the builtinpathmodule, e.g.:Describe alternatives you've considered
Please describe alternative solutions or features you have considered.
i could re-implement
./deps/npm/node_modules/minimatch/minimatch.jsinto my zero-dependency-project, but i suspect there are many other projects out there that would benefit from exposing this api.