fix: normalize exports in index.js

[charlag]

Rationale

Mutating or otherwise using module.exports causes issues with static analysis e.g. with @rollup/plugin-commonjs.
See this example: https://stackblitz.com/edit/github-zdltp46a-47bx3pea?file=dist%2Fbundled.js

Wait for it to run and open dist/bundled.js (warning: big file). Notice how rollup was only able to produce a default export that contains the whole namespace (in the end of the file).

After the fix: https://stackblitz.com/edit/github-zdltp46a-lhwu7t6z?file=README.md

Wait for it to run and open dist/bundles.js. Notice that there are individual export items, like expected (in the end of the file).

Changes

Normalize exports in index.js so that module.export properties are not used after assignment.

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• Documented
• Review ready
• In review
• Merge ready

[Uzlopak]

THere is no guarantee, without a test, that we dont modify this in the future and breaking your issue again.

How about require everything at the top and then assigned it accordingly and adding a comment describing that issue.

[charlag]

I can imagine that this can happen. I think it's a good idea.
Aside: I really wish it was distributed as esm.

[charlag]

I've applied your suggestion

[metcoder95]

Can you fix the lint issues, and possibly add a test for a regression?

[charlag]

Sorry about the formatting!
Do you have any hint on how I would go about adding a test for this? I could scan this file for certain patterns but I'm not sure how effective it would be.

[metcoder95]

Seems tests are red, around fetch global it seems.

About how to test it, we can do something similar what we have done for jest and possibly start testing that the outcome has the required exports when using rollup.

[Uzlopak]

@charlag
If you apply that suggested patch, the tests should pass

[charlag]

thank you, since I should add a tests as well I can only come back to it next week

[Uzlopak]

I would actually prefer that you apply that patch and to see the CI passing green.
Then we could actually discuss if we want to simply merge it as is and not wait for a potential test in the future, which maybe never comes or is too complex for the issue it fixes.

Also: install rollup to ensure that it works for a rollup specific issue? Maybe the bug is actually on rollup side, so we should open an issue to the corresponding rollup repo? What is with esbuild? ncc? etc..

So I would just say, make it able to be merged. Then we just solve it by convention and not via test.

[charlag]

I did deliberate on whether I should file it against the commonjs plugin instead but my experience tells me against it:

• their code isn't new and it already handles a lot of cases
• I imagine that the problem is impossible to solve in a general case
• I'm not sure how much effort they want to put in commonjs interop since it is treated as a legacy format (I tend to agree)

there are some things that I want to clarify with rollup but I think in the case we are fixing here their answer will be "it is not supported" so I would appreciate if we can fix it on this side for now. it used to work just a few versions ago, until the imports got changed.

[Uzlopak]

LGTM

[metcoder95]

I would actually prefer that you apply that patch and to see the CI passing green.
Then we could actually discuss if we want to simply merge it as is and not wait for a potential test in the future, which maybe never comes or is too complex for the issue it fixes.

Fine doing so, my thoughts were more into find a way to detect possible regressions; although is true that if this fix the issue but later a new appears, it might be more a Rolloup bug rather than ours.

[Uzlopak]

I tried to write a reasonable test. But it seems that it doesnt work.

From 3c817670942dbbef91be6e7fbbcd6f16110dab8a Mon Sep 17 00:00:00 2001
From: Aras Abbasi <[email protected]>
Date: Thu, 10 Jul 2025 11:01:40 +0200
Subject: [PATCH] add test

---
 .gitignore                      |  3 +++
 .npmignore                      |  3 +++
 eslint.config.js                |  3 ++-
 package.json                    |  2 ++
 test/bundlers/rollup/exports.js | 39 +++++++++++++++++++++++++++++++++
 5 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 test/bundlers/rollup/exports.js

diff --git a/.gitignore b/.gitignore
index b07f9365..9fac783c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -92,3 +92,6 @@ test/request-timeout.10mb.bin
 # Claude files
 CLAUDE.md
 .claude
+
+# Temporary files created by tests
+test/**/*.js.tmp
diff --git a/.npmignore b/.npmignore
index 461d334e..c3e91647 100644
--- a/.npmignore
+++ b/.npmignore
@@ -14,3 +14,6 @@ lib/llhttp/llhttp.wasm
 
 # File generated by /test/request-timeout.js
 test/request-timeout.10mb.bin
+
+# Temporary files created by tests
+test/**/*.js.tmp
diff --git a/eslint.config.js b/eslint.config.js
index 3c0cbc1c..5117a1c8 100644
--- a/eslint.config.js
+++ b/eslint.config.js
@@ -8,7 +8,8 @@ module.exports = [
       'lib/llhttp',
       'test/fixtures/wpt',
       'test/fixtures/cache-tests',
-      'undici-fetch.js'
+      'undici-fetch.js',
+      'test/**/*.js.tmp'
     ],
     noJsx: true,
     ts: true
diff --git a/package.json b/package.json
index 858f7478..588b08eb 100644
--- a/package.json
+++ b/package.json
@@ -109,6 +109,7 @@
   "devDependencies": {
     "@fastify/busboy": "3.1.1",
     "@matteo.collina/tspl": "^0.2.0",
+    "@rollup/plugin-commonjs": "^28.0.6",
     "@sinonjs/fake-timers": "^12.0.0",
     "@types/node": "^18.19.50",
     "abort-controller": "^3.0.0",
@@ -125,6 +126,7 @@
     "neostandard": "^0.12.0",
     "node-forge": "^1.3.1",
     "proxy": "^2.1.1",
+    "rollup": "^4.44.2",
     "tsd": "^0.32.0",
     "typescript": "^5.6.2",
     "ws": "^8.11.0"
diff --git a/test/bundlers/rollup/exports.js b/test/bundlers/rollup/exports.js
new file mode 100644
index 00000000..8465cf27
--- /dev/null
+++ b/test/bundlers/rollup/exports.js
@@ -0,0 +1,39 @@
+'use strict'
+
+const { test } = require('node:test')
+const assert = require('node:assert')
+const { resolve: pathResolve } = require('node:path')
+const { rollup } = require('rollup')
+const pluginCommonJs = require('@rollup/plugin-commonjs')
+
+test('check if rollup esm bundle exports all exports from unbundled undici', async (t) => {
+  const rollupInstance = await rollup({
+    input: pathResolve(__dirname, '../../../index.js'),
+    plugins: [pluginCommonJs()],
+    // Hide warnings about importing node modules
+    logLevel: 'silent'
+  })
+  const generatedRollupBundle = await rollupInstance.generate({
+    plugins: [pluginCommonJs()],
+    format: 'esm'
+  })
+
+  assert.strictEqual(generatedRollupBundle.output.length, 1, 'should have one output')
+  assert.strictEqual(generatedRollupBundle.output[0].type, 'chunk', 'output should be a chunk')
+  assert.strictEqual(generatedRollupBundle.output[0].fileName, 'index.js', 'output should be named index.js')
+
+  await rollupInstance.write({
+    plugins: [pluginCommonJs()],
+    file: pathResolve(__dirname, 'undici-rollup-esm-bundle.js.tmp'),
+    format: 'esm'
+  })
+
+  const unbundledUndici = require(pathResolve(__dirname, '../../../index.js'))
+  const bundledUndici = require(pathResolve(__dirname, 'undici-rollup-esm-bundle.js.tmp'))
+
+  assert.strictEqual(Object.keys(unbundledUndici).length, Object.keys(bundledUndici).length,
+    'bundled undici should have the same number of exports as unbundled undici')
+  for (const key of Object.keys(unbundledUndici)) {
+    assert.ok(bundledUndici[key], `bundled should have ${key}`)
+  }
+})
-- 
2.43.0

[charlag]

@Uzlopak hi, I tried your patch and it seems to work for me? I think it's not even necessary to write the bundle, just generating is enough and then checking output.output[0].code.

[Uzlopak]

Ok, strange. You can modify the Test if you like.

[charlag]

I am trying it out now and seems like something got changed in @rollup/plugin-commonjs 27, investigating

[charlag]

It seems like the plugin behavior has changed:
https://github.com/rollup/plugins/tree/master/packages/commonjs#using-commonjs-files-as-entry-points

With this plugin, you can also use CommonJS files as entry points. This means, however, that when you are bundling to an ES module, your bundle will only have a default export. If you want named exports instead, you should use an ES module entry point instead that reexports from your CommonJS entry point,

Which means to make this work with the plugin as-is undici would have to have an ESM entry point that lists all the named exports.

I will raise an issue with rollup but I also wanted to ask how undici maintainers feels about having an ESM entry point. commonjs entry point could just re-export from it to avoid having 2 versions.

[metcoder95]

Unless this provides common ground benefits account bundlers, and not only to patch rollup in specific, I'm ok with doing so; but in this case I'm starting to feel that is mostly a rollup problem that has to be fixed some how at their end or at least provide a common ground for it