-
Notifications
You must be signed in to change notification settings - Fork 71
chore(deps): switch from fast-glob to tinyglobby #6354
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
base: main
Are you sure you want to change the base?
Conversation
Hello @benmccann, I've just merged #6384 which replacing use of |
596fa49
to
7f62758
Compare
Unfortunately #6384 made this much harder because it introduced the use of the Failing tests``` ❯ tests/symlinked_included_files.test.ts:59:47 57| 58| // expect that the symlink for `node_modules/crazy-dep` is preserved 59| expect(await readDirWithType(unzippedPath)).toEqual({ | ^ 60| '___netlify-bootstrap.mjs': false, 61| '___netlify-entry-point.mjs': false,⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/3]⎯ FAIL tests/symlinked_included_files.test.ts > preserves multiple symlinks that link to the same target
❯ tests/symlinked_included_files.test.ts:109:59 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/3]⎯ FAIL tests/symlinked_included_files.test.ts > symlinks in subdir of
❯ tests/symlinked_included_files.test.ts:153:59 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[3/3]⎯
|
}) | ||
|
||
// tinyglobby ends directories with `/`, so we can filter them out using that |
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.
fyi this is not guaranteed and the opposite can be true in rare (but known) cases. probably best to not rely on trailing /
🎉 Thanks for submitting a pull request! 🎉
Summary
https://npmgraph.js.org/?q=fast-glob - 17 dependencies
https://npmgraph.js.org/?q=tinyglobby - 2 dependencies
I see #6094 tried to switch both
fast-glob
andglob
. However,tinyglobby
is currently only a replacement forglobby
andfast-glob
. I made a request forglob
support awhile back here: SuperchupuDev/tinyglobby#97. It might not be implemented anytime soon if at all, so it'd be nice to migrate justfast-glob
in the meantime, which is still a nice win in terms of dependency size.For us to review and ship your PR efficiently, please perform the following steps:
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
your code follows our style guide and passes our tests.
A picture of a cute animal (not mandatory, but encouraged)