Update dependencies, remove Glob:glob and replace with node:fs/promises:glob.#546
Update dependencies, remove Glob:glob and replace with node:fs/promises:glob.#546arcaniussainey wants to merge 6 commits intofastify:mainfrom
Conversation
Eomm
left a comment
There was a problem hiding this comment.
could you run npm run lint:fix and commit the changes?
|
Ran linter and committed changes, apologies for the delay! |
|
I'm also noting that with one of the errors, specifically caused by the following test: test('dir list default options', async t => {
t.plan(1)
const options = {
root: path.join(__dirname, '/static'),
prefix: '/public',
list: true
}
const route = '/public/shallow'
const content = { dirs: ['empty'], files: ['sample.jpg'] }
await helper.arrange(t, options, async (url) => {
await t.test(route, async t => {
t.plan(3)
const response = await fetch(url + route)
t.assert.ok(response.ok)
t.assert.deepStrictEqual(response.status, 200)
t.assert.deepStrictEqual(await response.json(), content)
})
})
})Outputting test at test\dir-list.test.js:123:13
✖ /public/shallow (66.4565ms)
AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
+ actual - expected
{
dirs: [
'empty'
],
files: [
+ 'no-link',
'sample.jpg'
]
}I note that the file, |
There was a problem hiding this comment.
Both fsPromises.glob and Array.fromAsync are documented as Node.js v22 features. CI still supports v20. Will this not break v20?
|
As documented here, this is something that we cannot achieve yet. |
|
I apologies, to me this thread is a bit confusing. Real issue here is that there is a security vulnerability in glob which cases issues and things like Snyk to fail: Quite easy to fix, looks like. Everything else, is totally different perspective and some kind of philosophical/architecture decision. Thank you for that, but can we address actual issue? Is there are plans to fix security vulnerability? No need to major refactoring/custom implementations whatsoever. |
I am not sure how it related to our project. |
I have removed
glob:globand replaced withnode/fs:glob. Code style has remained largely the same, however it seems there is still some test cases that are producing additional errors such as the following:This was decided via diffing the testcase output of the main branch and my PR. If I'm doing something stupid with the new version let me know and I'll fix it then resubmit.
This change will constrain releases to node >= 20, as node/fs:glob was added around then. I've included my testcase output below.