Repository navigation
Enhancement: replace globby with fast-glob #9453
Description
Before You File a Proposal Please Confirm You Have Done The Following...
- I have searched for related issues and found none that match my proposal.
- I have searched the current rule list and found no rules that match my proposal.
- I have read the FAQ and my problem is not listed.
Relevant Package
typescript-estree
My proposal is suitable for this project
- I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).
Description
Previously, typescript-eslint switched from glob to globby for tsconfig.json file searching: #2418
Under the hood, globby is powered by fast-glob with extra features, but none of them are utilized by the typescript-eslint.
Since the typescript-eslint doesn't utilize those extra features, I propose replacing globby with fast-glob to reduce the installation size.
Additional Info
The installation size difference
The installation size of globby is 605 KiB and 23 transitive dependencies are introduced, while the installation size of fast-glob is 503 KiB and 18 transitive dependencies are introduced.
Proof of Concept
I've tested the proposal in my fork: SukkaW@0cec339 and all existing test cases are passed on my machine.
globby's extra features
Here is globby's feature description:
- Promise API
fast-globalso has promise-based API, andtypescript-eslintonly uses the sync API
- Multiple patterns
globby's multiple patterns support is powered byfast-globdirectly
- Negated patterns:
['foo*', '!foobar']globby's negated patterns support is also powered byfast-globdirectly
- Expands directories:
foo → foo/**/*typescript-eslintdoesn't rely on this feature since the exact matches oftsconfig.json(-like) files are required
- Supports
.gitignoreand similar ignore config filestypescript-eslintdoesn't rely on this feature sinceglobbydisabled this feature by default andtypescript-eslintdoesn't enable it.
- Supports URL as
cwd- Currently
typescript-eslintpassed the string path tocwd, and we can use Node.js built-inurl.fileURLToPathin case we encounter URLs in the future.
- Currently
Activity
JoshuaKGoldberg commentedon Jun 29, 2024
Ooh nice spot! I'm in favor of this personally. 👍
SukkaW commentedon Jun 29, 2024
In case the proposal is accepted, I am wondering whether the change should be introduced to v8 only, or backport it to v7 as well. What do you think?
benmccann commentedon Jun 29, 2024
Both libraries are quite large. What about using something smaller like
fdir?Compare https://npmgraph.js.org/?q=fast-glob vs https://npmgraph.js.org/?q=fdir
See rollup/plugins#1741 for an example of switching
SukkaW commentedon Jun 29, 2024
The user-land config of
typescript-eslintsupports extended glob patterns, thus we have to choose one that has first-party glob pattern supports, wherepicomatchdoesn't have.fregante commentedon Jul 1, 2024
typescript-eslintis currently Node 18+. Once you reach Node 22 (end of the year?) you will be able to use the native globbing library for a total of 0k and 0 dependencies 😃bradzacher commentedon Jul 1, 2024
We (and the entire ecosystem really) won't drop node 18 till it's EOL - 2025-04-30.
Same for v20 - 2026-04-30.
https://github.com/nodejs/Release?tab=readme-ov-file#release-schedule
SukkaW commentedon Jul 1, 2024
Also, there might be some behavior differences between
fast-globand Node.js glob. And Node.js built-in glob is not stable yet.SuperchupuDev commentedon Jul 22, 2024
just my two cents: after weeks of trying to replace globby with lighter alternatives i've published
tinyglobbywhich only uses two subdependencies. maybe not outright switching to it but it could be an option once it gets more battle tested in other packagesbradzacher commentedon Aug 23, 2024
I don't really have a problem with switching out the underlying library for globbing.
If we're not using any of the features globby adds and there isn't any visible change for users in terms of supported glob patterns -- then we're free to do whatever without a major release.
If we can make things faster and / or smaller without too much effort then happy for it to be done!
bradzacher commentedon Aug 25, 2024
So we have #9518 and #9878.
Do we replace
globbywithtinyglobbyorfastglob.Is one better than the other? I don't have an opinion either way, personally.
cc @typescript-eslint/triage-team
SukkaW commentedon Aug 25, 2024
@bradzacher
tinyglobbyis more lightweight thanfast-glob, but I don't know which one is more performant.Since finding the corresponding
tsconfig.jsonfile for the current linting file is a hotpath, we might wanna merge this one first sincefast-globis consistently faster and more predictable (theglobbywe want to replace is powered byfast-globunder the hood, hence the behavior match). We can investigatetinyglobbylater.@benmccann What do you think?
benmccann commentedon Aug 25, 2024
fast-globhas 17 dependencies whereastinyglobbyhas 2. It varies based on the input, but from what I've seentinyglobbyis a bit faster thanfast-globbenmccann commentedon Aug 25, 2024
I've gotten the CI green now on #9878 based on including some of the changes from this PR. I think we could probably just merge that one since
tinyglobbyis both smaller and fasterI didn't include the change from #9518 (comment) since it was unrelated to switching glob libraries. I could update my PR to include it if desired or maybe that PR could be updated to just have that change or the change could be submitted as a new PR
JamesHenry commentedon Aug 25, 2024
fast-globis very well established and it's very common usage across other heavily used packages likely makes the dependency point less relevant in practice (they will be deduped a lot). I would personally rather go for an established library here. Maybe we can revisit in the future iftinyglobbygains wider adoption (it was literally created during the lifecycle of this issue so it's as nascent as it gets right now)benmccann commentedon Aug 25, 2024
While
tinyglobbyis pretty new, most of the logic is in more established libraries it uses as dependencies such aspicomatch, which is also used byfast-globunder the covers. Thetibyglobbycode is so small there's little room for bugs in it. And it's already used well over 100,000 times/week.fast-globwould have to be used in about 90% of projects with none usingtinyglobbyfor consumers to come out ahead from deduping since it has so many more dependencies. I actually havetinyglobbyin my dependency tree multiple times already because a number of other libraries have switched. Nuxt just switched to it a few days ago: https://github.com/nuxt/nuxt/pull/28686/filesbradzacher commentedon Aug 26, 2024
As a first step let's go to
fast-globas it's:globbyyarn.lock.In future I'm open to revisiting this and switching to
tinyglobbyinstead when it gains a bit broader adoption.But yeah let's go with one step-at-a-time.