Bundling undici with rollup results in circular dependency and node:sqlite(?) getting included

[soulchild]

Bug Description

I needed proxy support for node's built-in fetch. So I installed undici as a dependency and used setGlobalDispatcher together with ProxyAgent. Afterwards my rollup build failed with:

> rollup -c
./index.js → test...
(!) Unresolved dependencies
https://rollupjs.org/troubleshooting/#warning-treating-module-as-external-dependency
node:sqlite (imported by "node:sqlite?commonjs-external")
(!) Circular dependency
node_modules/undici/lib/web/websocket/util.js -> node_modules/undici/lib/web/websocket/connection.js -> node_modules/undici/lib/web/websocket/util.js

It also seems that node:sqlite is included in the final bundle. Do I need to configure rollup differently or is this a bug? I had no bundling problems before introducing undici…

Reproducible By

Clone https://github.com/soulchild/undici-rollup-bundle-failure and run npm run build.

Expected Behavior

rollup should be able to bundle undici without problems … and not include node:sqlite 😉

Environment

• macOS 15.2
• Node v22.10.0

[mcollina]

I 100% agree on the point of the circular dependency.

The node:sqlite module is from Node.js core, and undici only runs on Node.js. It shouldn't be bundled in the way node:events or node:http is bundled - you would like want to report that to rollup because it's a bug on their end - we cannot do anything about it.

[soulchild]

Thanks for the quick response, @mcollina. I'll raise an issue for Rollup then. 😉

[soulchild]

For reference: rollup/rollup#5800

[mcollina]

We should solve the circular dependency here.

[mcollina]

#4006 fixes the circular dependency

[soulchild]

Wow, that was fast! Thanks a lot!

[soulchild]

@mcollina, I noticed that including undici just to add proxy support to fetch increases the size of the final bundle by almost 500kb. That's quite unfortunate for my use-case (a CLI tool which, without undici, is just 90 kb).

TBH, proxy support in Node packages has always been an afterthought and every time I'm tasked with adding it, I know for sure I'm gonna run into weird edge-cases and will be spending the next two days banging my head against the wall. Is there maybe something we could do about it? 🙏

I updated the repository with a working rolldown example (which doesn't have the bundling problem like rollup) and also included the final bundle sizes in the README.md.

[mcollina]

Frankly we should just add it out of the box if the right headers are set (both here and in Node.js). In other terms, if HTTP_PROXY, HTTPS_PROXY are set, add https://github.com/nodejs/undici/blob/7f635e51f6170f4b59abedc7cb45e6bcda7f056d/lib/dispatcher/env-http-proxy-agent.js in

undici/lib/global.js

Line 10 in 7f635e5

setGlobalDispatcher(new Agent())

. If you'd like to do a PR on that front, that'd be interesting to discuss.

There is also nodejs/node#43187 (requires a champion and it's likely a lot of work).

[soulchild]

Frankly we should just add it out of the box if the right headers are set (both here and in Node.js).

That'd be much appreciated! It seems like EnvHttpProxyAgent already does the right thing: Depending on whether proxy-related environment variables exist, it uses ProxyAgent and if not, falls back to the default Agent. So, shouldn't it be enough to just replace

undici/lib/global.js

Line 10 in 7f635e5

setGlobalDispatcher(new Agent())

with

setGlobalDispatcher(new EnvHttpProxyAgent())

Or would we rather check for the existence of the proxy environment variables again inside global.js and instantiate one or the other agent?

[metcoder95]

EnvHttpProxyAgent already does the environment check, so it should be a matter of just replacing one with the other

[soulchild]

@mcollina What do you think? Shall we reopen this issue or create a new one regarding out-of-the-box proxy support?