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

override whatwg-url dependency #1953

Merged
merged 2 commits into from
Oct 20, 2024

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Oct 18, 2024

The generated client on the release-1.x branch relies on the node-fetch@2.x module, which depends on whatwg-url@5.0.0, which uses the Node core punycode module. Node's punycode module was runtime deprecated in v21, which causes this module to print a deprecation warning when used on newer versions of Node.

whatwg-url@9.0.0 stopped using punycode directly, but continued to use it indirectly via its own tr46 dependency. The problematic use of punycode was finally removed in whatwg-url@12.0.1.

node-fetch@2.x claims backwards compatibility to Node v4, but the fixed version of whatwg-url only claims compatibility back to Node 12. For this reason, the node-fetch project has stated that they will not address this issue. For reference, Node v4 went EOL in 2018, and Node v12 when EOL in early 2022. Node 18 is currently the oldest supported version, and Node v23 was released this week.

Ideally, the generator will move to a newer version of node-fetch, native fetch, or even the undici module that implements fetch in core. Until that happens, this module can override whatwg-url and silence the deprecation warning.

It's also worth noting that this punycode deprecation is not the same one as the punycode deprecation on the master branch.

This change has been tested locally on v18.0.0.

Refs: nodejs/node#47202
Refs: node-fetch/node-fetch#1793

(Sorry for the excessive commit message, but there is a lot of context to capture and maybe it will help someone looking through the git history one day.)

The generated client on the release-1.x branch relies on the
node-fetch@2.x module, which depends on whatwg-url@5.0.0, which
uses the Node core punycode module. Node's punycode module was
runtime deprecated in v21, which causes this module to print a
deprecation warning when used on newer versions of Node.

whatwg-url@9.0.0 stopped using punycode directly, but continued
to use it indirectly via its own tr46 dependency. The problematic
use of punycode was finally removed in whatwg-url@12.0.1.

node-fetch@2.x claims backwards compatibility to Node v4, but the
fixed version of whatwg-url only claims compatibility back to
Node 12. For this reason, the node-fetch project has stated that
they will not address this issue. For reference, Node v4 went
EOL in 2018, and Node v12 when EOL in early 2022. Node 18 is
currently the oldest supported version, and Node v23 was released
this week.

Ideally, the generator will move to a newer version of node-fetch,
native fetch, or even the undici module that implements fetch in
core. Until that happens, this module can override whatwg-url
and silence the deprecation warning.

It's also worth noting that this punycode deprecation is not the
same one as the punycode deprecation on the master branch.

This change has been tested locally on v18.0.0.

Refs: nodejs/node#47202
Refs: node-fetch/node-fetch#1793
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 18, 2024
@brendandburns
Copy link
Contributor

/lgtm
/approve

I hope that Dependabot continues to update this dependency...

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 19, 2024
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 19, 2024
@cjihrig
Copy link
Contributor Author

cjihrig commented Oct 19, 2024

I hope that Dependabot continues to update this dependency

I'm not 100% sure how Dependabot and overrides might work together. However, prior to this change, whatwg-url was at 5.0.0, while the latest on npm is 14.0.0. node-fetch won't be updating it on their end per the linked issue.

@brendandburns
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 20, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, cjihrig

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit ca8ccf6 into kubernetes-client:release-1.x Oct 20, 2024
7 checks passed
@cjihrig cjihrig deleted the punycode branch October 20, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants