-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
chore(deps)!: migrate fast-glob to tinyglobby
#18243
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
Conversation
Only two files changed in this commit as we need to wait for `tinyglobby` to support some currently unsupported options.
|
|
| : [`**/__tests__/**`, `**/coverage/**`]), | ||
| ], | ||
| absolute: true, | ||
| suppressErrors: true, // suppress EACCES errors |
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.
Such errors are suppressed by default in tinyglobby.
| --------------------------------------- | ||
|
|
||
| ## fdir |
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.
Should fast-glob be removed? Or does that depend on my PR to the rollup plugins?
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.
It's still bundled because the dynamic import plugin depends on it. If your PR doesn't get merged before v6 release, I think we can patch it since we only use the dynamicImportToGlob function.
fast-glob to tinyglobbyfast-glob to tinyglobby
sapphi-red
left a comment
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.
Thanks!
bluwy
left a comment
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.
I checked @rollup/plugin-dynamic-import-vars and it looks like the dynamicImportToGlob API that we only use still indirectly relies on fast-glob escapePath, so we actually need to have upstream swap to tinyglobby to deduplicate it, or fork the code locally as I guess it could take a while before the changes are accepted upstream. Maybe we can followup with that separately.
There's a PR open in the upstream to make the migration, but as you said, it might take a while before it's merged and released: rollup/plugins#1780. |
|
|
|
/ecosystem-ci run |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
📝 Ran ecosystem CI on
✅ analogjs, histoire, ladle, laravel, marko, nuxt, previewjs, quasar, qwik, rakkas, redwoodjs, storybook, vite-environment-examples, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress, vuepress |
Vite did similar: vitejs/vite#18243
This is to stay consistent with vite itself: vitejs/vite#18243 The change in the tests is necessary due to a known inconsistency between the libraries for that specific use case: SuperchupuDev/tinyglobby#53 Resolves #6
Description
As discussed in the Vite Discord (https://discord.com/channels/804011606160703521/804439875226173480/1282752023930081310), this PR is a suggestion to replace heavier
fast-globwith lightertinyglobby. The supported patterns are the same except for the incremented braces.