feat: swapping our node-fetch for unidici / native

[nicholasgriffintn]

• Replaces direct node-fetch imports with undici and removes dependencies
• Moves proxy handling from agent to dispatcher
• Removes GitLab fetch shim and upgrades nock
• Updates tests

Note:

Unidici v8 drops support for node 20 and v7 dropped support for node 18.

It would be best to be on the latest version but that would require Danger JS to presumably also drop support for 18 and 20.

[fbartho]

Since this is a pretty high sensitivity spot for code change I look forward to further reviews from participants.

Thanks for starting the process!

[fbartho]

https://www.npmjs.com/package/undici This seems to suggest that the version you selected is several major versions behind. (unless I’m confused?).

I’m not super familiar with undici, but if we’re swapping out dependencies, I would have expected to be on the latest version.

[nicholasgriffintn]

Sorry, I should have wrote the reason for that, I forgot to. I'll update the description.

Unidici v8 drops support for node 20 and v7 dropped support for node 18.

It would be best to be on the latest version but that would require Danger JS to presumably also drop support for 18 and 20.

[fbartho]

Huh! Not my decision to make, but this might be enough for me to suggest moving to node v24 as our minimum?

[nicholasgriffintn]

Yeah happy to update it, just working from the current support that's documented in the package.

[orta]

I think this is worth keeping lower, backwards compat is pretty important to this project 👍🏻

[glensc]

but it shouldn't be pinned to certain version, but use semver range: "undici": "^6.21.1"

[nicholasgriffintn]

Personally, I always use pinned to avoid supply chain attacks and then allow automations to upgrade via PRs like dependabot.

[glensc]

You shouldn't pin in libraries, as then downstream apps can't upgrade, but rather require library to update pin again.

However, this concrete dependency was already upgraded due same reason, that the pinned version needs security update: #1518, but it has no effect downstream, until another danger-js npm release is made.

[nicholasgriffintn]

not strictly true, sure thats a usability concern, but doing so is causing security concerns its just which one you see as more of a problem.

but sure, seems to have been changed, my default is always to pin nowadays, outside of peer deps.

[glensc]

pinning in libraries moves burden of updating dependency to library maintainer. if library uses version range, you can update dependency in your app.

surely, certain package mangers have "overrides" to overcome pinned version in library, but that goes more to danger zone, as there's no validation what version you pick. could be totally incompatible version.

[orta]

TBH, I'm pretty down for a 'cleanup for node 24 style thing for Danger JS. Personally, I've been moving all my projects to use risk instead.

[orta]

LEts do this