fix: Explicitly named exports should be exported over export * (#103)

Closes #102

Author: timfish

hook.js

@@ -129,15 +129,32 @@ 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)
+      }
+    } else {
+      // Store export * exports so we know they can be overridden by explicit
+      // named exports
+      if (isStarExport) {
+        starExports.add(name)
+      }
+
       setters.set(name, setter)
     }
   }
@@ -165,7 +182,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, `

test/fixtures/duplicate-b.mjs

@@ -1 +1 @@
-export const foo = 'a'
+export const foo = 'b'

test/fixtures/duplicate-c.mjs

@@ -0,0 +1 @@
+export const foo = 'c'

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'

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')

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')

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)