From ccc71ec0037135bc7cf249871aaf6e5265263f87 Mon Sep 17 00:00:00 2001
From: Tim Fish <tim@timfish.uk>
Date: Wed, 12 Jun 2024 17:50:36 +0200
Subject: [PATCH 1/2] fix: Explicitly named exports should be exported

---
 hook.js                                  | 34 +++++++++++++++++++-----
 test/fixtures/duplicate-b.mjs            |  2 +-
 test/fixtures/duplicate-c.mjs            |  1 +
 test/fixtures/duplicate-explicit.mjs     |  5 ++++
 test/hook/duplicate-exports-explicit.mjs | 13 +++++++++
 test/hook/duplicate-exports.mjs          |  6 ++---
 test/hook/v18.19-openai.mjs              |  6 +++++
 7 files changed, 56 insertions(+), 11 deletions(-)
 create mode 100644 test/fixtures/duplicate-c.mjs
 create mode 100644 test/fixtures/duplicate-explicit.mjs
 create mode 100644 test/hook/duplicate-exports-explicit.mjs

diff --git a/hook.js b/hook.js
index 4ec7679..c60daff 100644
--- a/hook.js
+++ b/hook.js
@@ -129,15 +129,35 @@ function isBareSpecifier (specifier) {
  */
 async function processModule ({ srcUrl, context, parentGetSource, parentResolve, excludeDefault }) {
   const exportNames = await getExports(srcUrl, context, parentGetSource)
-  const duplicates = new Set()
+  const starExports = new Set()
   const setters = new Map()
 
-  const addSetter = (name, setter) => {
-    // When doing an `import *` duplicates become undefined, so do the same
+  const addSetter = (name, setter, isStarExport = false) => {
     if (setters.has(name)) {
-      duplicates.add(name)
-      setters.delete(name)
-    } else if (!duplicates.has(name)) {
+      if (isStarExport) {
+        // If there's already a matching star export, delete it
+        if (starExports.has(name)) {
+          setters.delete(name)
+        }
+        // and return so this is excluded
+        return
+      }
+
+      // if we already have this export but it is from a * export, overwrite it
+      if (starExports.has(name)) {
+        starExports.delete(name)
+        setters.set(name, setter)
+      }
+
+      // We can only get here if there are two explicitly named exports which is
+      // not valid ESM
+    } else {
+      // Store export * exports so we know they can be overridden by explicit
+      // named exports
+      if (isStarExport) {
+        starExports.add(name)
+      }
+
       setters.set(name, setter)
     }
   }
@@ -164,7 +184,7 @@ async function processModule ({ srcUrl, context, parentGetSource, parentResolve,
         excludeDefault: true
       })
       for (const [name, setter] of setters.entries()) {
-        addSetter(name, setter)
+        addSetter(name, setter, true)
       }
     } else {
       addSetter(n, `
diff --git a/test/fixtures/duplicate-b.mjs b/test/fixtures/duplicate-b.mjs
index a41341d..853627d 100644
--- a/test/fixtures/duplicate-b.mjs
+++ b/test/fixtures/duplicate-b.mjs
@@ -1 +1 @@
-export const foo = 'a'
+export const foo = 'b'
diff --git a/test/fixtures/duplicate-c.mjs b/test/fixtures/duplicate-c.mjs
new file mode 100644
index 0000000..1f2f80e
--- /dev/null
+++ b/test/fixtures/duplicate-c.mjs
@@ -0,0 +1 @@
+export const foo = 'c'
diff --git a/test/fixtures/duplicate-explicit.mjs b/test/fixtures/duplicate-explicit.mjs
new file mode 100644
index 0000000..45514e5
--- /dev/null
+++ b/test/fixtures/duplicate-explicit.mjs
@@ -0,0 +1,5 @@
+export * from './duplicate-a.mjs'
+export * from './duplicate-b.mjs'
+export { foo } from './duplicate-c.mjs'
+export * from './duplicate-a.mjs'
+export * from './duplicate-b.mjs'
diff --git a/test/hook/duplicate-exports-explicit.mjs b/test/hook/duplicate-exports-explicit.mjs
new file mode 100644
index 0000000..cc97216
--- /dev/null
+++ b/test/hook/duplicate-exports-explicit.mjs
@@ -0,0 +1,13 @@
+import * as lib from '../fixtures/duplicate-explicit.mjs'
+import { strictEqual } from 'assert'
+import Hook from '../../index.js'
+
+Hook((exports, name) => {
+  if (name.endsWith('duplicate-explicit.mjs')) {
+    strictEqual(exports.foo, 'c')
+    exports.foo += '-wrapped'
+  }
+})
+
+// foo should not be exported because there are duplicate exports
+strictEqual(lib.foo, 'c-wrapped')
diff --git a/test/hook/duplicate-exports.mjs b/test/hook/duplicate-exports.mjs
index 2011602..2f585f5 100644
--- a/test/hook/duplicate-exports.mjs
+++ b/test/hook/duplicate-exports.mjs
@@ -4,8 +4,8 @@ import Hook from '../../index.js'
 
 Hook((exports, name) => {
   if (name.match(/duplicate\.mjs/)) {
-    // foo should not be exported because there are duplicate exports
-    strictEqual(exports.foo, undefined)
+    // foo should not be exported because there are duplicate * exports
+    strictEqual('foo' in exports, false)
     // default should be exported
     strictEqual(exports.default, 'foo')
   }
@@ -14,6 +14,6 @@ Hook((exports, name) => {
 notEqual(lib, undefined)
 
 // foo should not be exported because there are duplicate exports
-strictEqual(lib.foo, undefined)
+strictEqual('foo' in lib, false)
 // default should be exported
 strictEqual(lib.default, 'foo')
diff --git a/test/hook/v18.19-openai.mjs b/test/hook/v18.19-openai.mjs
index 9e0dcb6..9e0eb1d 100644
--- a/test/hook/v18.19-openai.mjs
+++ b/test/hook/v18.19-openai.mjs
@@ -8,3 +8,9 @@ Hook((exports, name) => {
 })
 
 console.assert(OpenAI)
+
+const openAI = new OpenAI({
+  apiKey: 'doesnt-matter'
+})
+
+console.assert(openAI)

From 55cc406f3471a8b77fb1afc49043e5b0ddec0161 Mon Sep 17 00:00:00 2001
From: Tim Fish <tim@timfish.uk>
Date: Wed, 12 Jun 2024 17:52:06 +0200
Subject: [PATCH 2/2] trim comment

---
 hook.js | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hook.js b/hook.js
index c60daff..79caecd 100644
--- a/hook.js
+++ b/hook.js
@@ -148,9 +148,6 @@ async function processModule ({ srcUrl, context, parentGetSource, parentResolve,
         starExports.delete(name)
         setters.set(name, setter)
       }
-
-      // We can only get here if there are two explicitly named exports which is
-      // not valid ESM
     } else {
       // Store export * exports so we know they can be overridden by explicit
       // named exports