-
Notifications
You must be signed in to change notification settings - Fork 319
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
Use native Fetch API in behaviour tests #3045
Conversation
@@ -18,12 +17,6 @@ const expectedPages = [ | |||
'/examples/template-custom' | |||
] | |||
|
|||
// Reduce test keep-alive | |||
setGlobalDispatcher(new Agent({ | |||
keepAliveTimeout: 10, |
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.
Unlike the undici
Writing tests docs there is no keepalive default
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.
Looks good to me! Love to see native stuff being used in favour of packages.
My only comment is definitely non-blocking.
"govuk_frontend_toolkit": "^9.0.1", | ||
"govuk_template_jinja": "^0.26.0", | ||
"govuk-elements-sass": "3.1.3", |
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.
Nice little tidy up.
I would've expected govuk-elements-sass
to stay where it was since hyphens come before underscores in ASCII/UTF (think it also just makes sense from a human-readable point of view - "e" comes before "f", and all 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.
@domoscargin I'll put my hand up and say I manually ordered it wrong last time (sorry), but this time it was definitely npm fixing my mistake so I've trusted it here 😬
There's an ESLint plugin for keeping package.json
files in order, maybe we need it 🙈
Thanks @domoscargin
We temporarily used
undici
to prepare for the Fetch API landing in Node.js 18:request
package with Fetch API #2858Both PRs have now merged so we can remove
undici
(Another package Dependabot doesn't need to worry about)