Skip to content
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

Add support for Node.js 16 & 18 #3792

Merged
merged 2 commits into from May 13, 2022
Merged

Conversation

devoto13
Copy link
Collaborator

build: add Node 16 and 18 to the CI matrix

Update documentation, so that it does not become outdated whenever a new version of Node is released.

Test on the current version of Node, so we can spot problems early, but don't claim to support it as current release line sometimes introduces bugs, which are later fixed by Node itself.

Fixes #3728


fix: prefer IPv4 addresses when resolving domains

Node 17+ changed the DNS resolution (see nodejs/node#40702), so now it resolves localhost according to the OS settings instead of IPv4-address first. The Karma server only listens on IPv4 address (127.0.0.1) by default, but the requests are sent to localhost in several places and localhost is resolved into IPv6 address (::) in Node 17+. So the run/stop/proxy request is unable to reach the Karma server and produces an error. This commit configures karma to use the IPv4-address first approach in newer Node version as well.

In the future major release, we may consider changing defaults to listen on IPv6 address instead, but IPv6 is not supported in Docker on macOS and Windows, so I think we should not rush such a change to make sure karma works there out of the box.

Fixes #3730

Update documentation, so that it does not become outdated whenever a new version of Node is released.

Test on the current version of Node, so we can spot problems early, but don't claim to support it as current release line sometimes introduces bugs, which are later fixed by Node itself.

Fixes karma-runner#3728
@devoto13 devoto13 requested a review from jginsburgn May 10, 2022 18:56
lib/middleware/proxy.js Outdated Show resolved Hide resolved
lib/runner.js Outdated Show resolved Hide resolved
Copy link
Member

@jginsburgn jginsburgn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make the reason of this change more prominent in the code?

@jginsburgn
Copy link
Member

I made the Node.js 16 and 18 checks "Required".

Node 17+ changed the DNS resolution (see nodejs/node#40702), so now it resolves `localhost` according to the OS settings instead of IPv4-address first. The Karma server only listens on IPv4 address (127.0.0.1) by default, but the requests are sent to `localhost` in several places and `localhost` is resolved into IPv6 address (`::`) in Node 17+. So the run/stop/proxy request is unable to reach the Karma server and produces an error. This commit configures karma to use the IPv4-address first approach in newer Node version as well.

In the future major release, we may consider changing defaults to listen on IPv6 address instead, but IPv6 is not supported in Docker on macOS and Windows, so I think we should not rush such a change to make sure karma works there out of the box.

Fixes karma-runner#3730
@jginsburgn jginsburgn merged commit e17698f into karma-runner:master May 13, 2022
@karmarunnerbot
Copy link
Member

🎉 This PR is included in version 6.3.20 🎉

The release is available on:

Your semantic-release bot 📦🚀

@devoto13 devoto13 deleted the node-17 branch May 23, 2022 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants