-
Notifications
You must be signed in to change notification settings - Fork 61
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
fix(deps): replace noisy deprecated glob@8 with tinyglobby #6094
base: main
Are you sure you want to change the base?
Conversation
e38f12d
to
8174622
Compare
8174622
to
4e392bf
Compare
b30b386
to
c6969cd
Compare
const untranspiledFiles = await pGlob(`${path}/**/*.ts`) | ||
const untranspiledFiles = await glob(`${path}/**/*.ts`, { | ||
onlyFiles: true, | ||
followSymbolicLinks: false, |
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.
Without this, this glob()
call hangs — seemingly because it's traversing the whole filesystem 😬
@@ -10,7 +10,8 @@ export const getTreeFiles = async function (srcPath: string, stat: Stats): Promi | |||
|
|||
return await glob(`${srcPath}/**`, { | |||
ignore: `${srcPath}/**/node_modules/**`, | |||
nodir: true, | |||
followSymbolicLinks: false, |
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.
adding this here and in ./published.ts
fixes some tests in tests/main.test.ts
that otherwise hang indefinitely (see other 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.
Although even with this change (which I'm not sure is desirable... we do support symlinks I believe) there are three failing (not hanging) tests. To run those locally:
npm run build && nx run @netlify/zip-it-and-ship-it:test:ci -- tests/main.test.ts
Installing netlify-cli prints a deprecation warning: netlify/cli#7029. I was going to upgrade `glob`, but then I remembered it's a fairly large library, so why not use this as an opportunity to replace it? See https://github.com/es-tooling/module-replacements/blob/main/docs/modules/glob.md. Compare the APIs: - https://github.com/isaacs/node-glob#readme - https://github.com/SuperchupuDev/tinyglobby?tab=readme-ov-file#api The only differences are option names, which I've updated. Our uses either hardcode patterns or enforce precisely documented user patterns, so we won't hit any of the limitations of tinyglobby.
8aeaf96
to
3514a13
Compare
Summary
Installing netlify-cli prints a deprecation warning. Fixes netlify/cli#7029.
I was going to upgrade
glob
, but then I remembered it's a fairly large library, so why not use this as an opportunity to replace it? Seehttps://github.com/es-tooling/module-replacements/blob/main/docs/modules/glob.md.
Compare the APIs:
The only differences are option names and option defaults, which I've updated.
Our uses either hardcode patterns or enforce precisely documented user patterns, so we won't hit any of the limitations of tinyglobby.
... Then I also saw we're using
fast-glob
too, so I replaced that as well.Related
See also alternative PR upgrading
glob
instead: #6143.