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

Bump undici version and minimum node version #333

Merged
merged 4 commits into from
Aug 15, 2022
Merged

Bump undici version and minimum node version #333

merged 4 commits into from
Aug 15, 2022

Conversation

cameron-robey
Copy link
Contributor

To target security vulnerabilities in undici, we update to the latest version.

From undici v5.7.0, we require a node >=16.8.0 - here we update the minimum to 16.13.0 (the first Node 16 LTS release)

To target security vulnerabilities in undici, we update to the latest version.

From undici v5.7.0, we require a node >=16.8.0 - here we update the minimum to 16.13.0 (the first Node 16 LTS release)
We should pin the version of undici
Copy link

@JacobMGEvans JacobMGEvans left a comment

Choose a reason for hiding this comment

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

As long as @mrbbot is good with these changes, I think it is good to go.

@cameron-robey
Copy link
Contributor Author

Just worth noting that this will require some people to bump their version of node - I assume we have some precedent for a release like this

Copy link
Contributor

@mrbbot mrbbot left a comment

Choose a reason for hiding this comment

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

Nice, thank you! 🙂 Added a couple tiny commits to fix tests, but otherwise... ✅

@mrbbot mrbbot merged commit 7894fa4 into cloudflare:master Aug 15, 2022
@mrbbot mrbbot mentioned this pull request Aug 15, 2022
@cameron-robey cameron-robey deleted the undici-version branch August 16, 2022 11:43
@threepointone
Copy link
Contributor

Would you be ok using 16.8.0 as the minimum version instead? Makes the breaking change radius a little smaller.

@mrbbot
Copy link
Contributor

mrbbot commented Aug 22, 2022

The thinking here was what if undici does this again? I'd rather we didn't have to make this kind of breaking change twice, and 16.13.0 was the first long-term support version of Node 16. We'd like people to be using the latest, supported versions of Node anyways, and there were just over 2 months between the releases of 16.7.0 and 16.13.0 so I don't think this will cause too many issues. When it does, they'll be a clear error telling people what's wrong. We, as developers of the packages, also get the option of using the newly added features ourselves if needed.

@threepointone
Copy link
Contributor

That's fair. Let's land this?

@threepointone
Copy link
Contributor

wait it was landed already ha

@mrbbot
Copy link
Contributor

mrbbot commented Aug 22, 2022

...and released in 2.7.0 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants