-
-
Notifications
You must be signed in to change notification settings - Fork 110
Update dependencies, remove Glob:glob and replace with node:fs/promises:glob. #546
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
Eomm
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both fsPromises.glob and Array.fromAsync are documented as Node.js v22 features. CI still supports v20. Will this not break v20?
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.
yes it will...
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.