fix: treat default condition as esm or cjs (#657)

Co-authored-by: Jiachi Liu <inbox@huozhi.im>
huozhi · Feb 15, 2025 · dd38370 · dd38370
1 parent 1002f00
commit dd38370
Show file tree

Hide file tree

Showing 14 changed files with 299 additions and 45 deletions.
diff --git a/src/entries.ts b/src/entries.ts
@@ -241,8 +241,6 @@ export function getSpecialExportTypeFromConditionNames(
   conditionNames.forEach((value) => {
     if (specialExportConventions.has(value)) {
       exportType = value
-    } else if (value === 'import' || value === 'require' || value === 'types') {
-      // exportType = value
     }
   })
   return exportType

diff --git a/src/plugins/alias-plugin.ts b/src/plugins/alias-plugin.ts
@@ -26,6 +26,7 @@ function findJsBundlePathCallback(
     conditionNames: Set<string>
   },
   specialCondition: string,
+  isESMPkg: boolean,
 ): boolean {
   const hasBundle = bundlePath != null
   const formatCond = format === 'cjs' ? 'require' : 'import'
@@ -37,6 +38,20 @@ function findJsBundlePathCallback(
   // Check if the format condition is matched:
   // if there's condition existed, check if the format condition is matched;
   // if there's no condition, just return true, assuming format doesn't matter;
+  const bundleFormat = bundlePath.endsWith('.mjs')
+    ? 'esm'
+    : bundlePath.endsWith('.cjs')
+      ? 'cjs'
+      : isESMPkg
+        ? 'esm'
+        : 'cjs'
+
+  // If there's only default condition, and the format is matched
+  const isDefaultOnlyCondition =
+    conditionNames.size === 1 && conditionNames.has('default')
+      ? bundleFormat === format
+      : true
+
   const isMatchedFormat = hasFormatCond ? conditionNames.has(formatCond) : true
 
   const isMatchedConditionWithFormat =
@@ -47,7 +62,9 @@ function findJsBundlePathCallback(
     isMatchedConditionWithFormat &&
     !isTypesCondName &&
     hasBundle &&
-    isMatchedFormat
+    isMatchedFormat &&
+    isDefaultOnlyCondition
+
   if (!match) {
     const fallback = runtimeExportConventionsFallback.get(specialCondition)
     if (!fallback) {
@@ -59,9 +76,9 @@ function findJsBundlePathCallback(
       // The last guard condition is to ensure bundle condition but not types file.
       return (
         isMatchedFormat &&
+        !isTypesCondName &&
         (conditionNames.has(specialCondition) ||
-          fallback.some((name) => conditionNames.has(name))) &&
-        !conditionNames.has('types')
+          fallback.some((name) => conditionNames.has(name)))
       )
     }
   } else {
@@ -93,13 +110,15 @@ export function aliasEntries({
   entry: sourceFilePath,
   conditionNames,
   entries,
+  isESMPkg,
   format,
   dts,
   cwd,
 }: {
   entry: string
   entries: Entries
   format: OutputOptions['format']
+  isESMPkg: boolean
   conditionNames: Set<string>
   dts: boolean
   cwd: string
@@ -111,11 +130,17 @@ export function aliasEntries({
   for (const [, exportCondition] of Object.entries(entries)) {
     const exportDistMaps = exportCondition.export
     const exportMapEntries = Object.entries(exportDistMaps).map(
-      ([composedKey, bundlePath]) => ({
-        conditionNames: new Set(composedKey.split('.')),
-        bundlePath,
-        format,
-      }),
+      ([composedKey, bundlePath]) => {
+        const conditionNames = new Set(composedKey.split('.'))
+
+        return {
+          conditionNames,
+          bundlePath,
+          format,
+          isDefaultCondition:
+            conditionNames.size === 1 && conditionNames.has('default'),
+        }
+      },
     )
 
     let matchedBundlePath: string | undefined
@@ -135,22 +160,25 @@ export function aliasEntries({
         })?.bundlePath
       }
     } else {
-      matchedBundlePath = exportMapEntries
-        .sort(
-          // always put special condition after the general condition (default, cjs, esm)
-          (a, b) => {
-            if (a.conditionNames.has(specialCondition)) {
-              return -1
-            }
-            if (b.conditionNames.has(specialCondition)) {
-              return 1
-            }
-            return 0
-          },
+      const orderedExportConditions = exportMapEntries.sort((condA, condB) => {
+        const bHasSpecialCond = condB.conditionNames.has(specialCondition)
+        const aHasSpecialCond = condA.conditionNames.has(specialCondition)
+        if (bHasSpecialCond || aHasSpecialCond) {
+          const specialCompare =
+            Number(bHasSpecialCond) - Number(aHasSpecialCond)
+          if (specialCompare !== 0) {
+            return specialCompare
+          }
+        }
+
+        // Always put default condition at the end.
+        return (
+          Number(condA.isDefaultCondition) - Number(condB.isDefaultCondition)
         )
-        .find((item) => {
-          return findJsBundlePathCallback(item, specialCondition)
-        })?.bundlePath
+      })
+      matchedBundlePath = orderedExportConditions.find((item) => {
+        return findJsBundlePathCallback(item, specialCondition, isESMPkg)
+      })?.bundlePath
     }
 
     if (matchedBundlePath) {

diff --git a/src/rollup/input.ts b/src/rollup/input.ts
@@ -194,6 +194,7 @@ export async function buildInputConfig(
     entry,
     entries,
     format: aliasFormat,
+    isESMPkg: isESModulePackage(pkg.type),
     conditionNames: new Set(currentConditionNames.split('.')),
     dts,
     cwd,

diff --git a/test/integration/default-default-export-different-ext/index.test.ts b/test/integration/default-default-export-different-ext/index.test.ts
@@ -0,0 +1,67 @@
+import { describe, expect, it } from 'vitest'
+import {
+  getFileNamesFromDirectory,
+  createJob,
+  getFileContents,
+} from '../../testing-utils'
+
+describe('integration - default-default-export-different-ext', () => {
+  const { distDir } = createJob({ directory: __dirname })
+
+  it('should not export esm module in cjs file', async () => {
+    const distFiles = await getFileNamesFromDirectory(distDir)
+    expect(distFiles).toEqual([
+      'a.cjs',
+      'a.d.ts',
+      'a.js',
+      'b.cjs',
+      'b.d.ts',
+      'b.js',
+      'c.cjs',
+    ])
+
+    const contents = await getFileContents(distDir)
+    // TODO: add to contents
+    for (const file in contents) {
+      if (file.includes('.d.')) {
+        delete contents[file]
+      }
+    }
+    expect(contents).toMatchInlineSnapshot(`
+      {
+        "a.cjs": "var b_cjs = require('./b.cjs');
+
+      const a = 'a';
+
+      exports.a = a;
+      Object.keys(b_cjs).forEach(function (k) {
+      	if (k !== 'default' && !Object.prototype.hasOwnProperty.call(exports, k)) Object.defineProperty(exports, k, {
+      		enumerable: true,
+      		get: function () { return b_cjs[k]; }
+      	});
+      });
+      ",
+        "a.js": "export * from './b.js';
+
+      const a = 'a';
+
+      export { a };
+      ",
+        "b.cjs": "const b = 'b';
+
+      exports.b = b;
+      ",
+        "b.js": "const b = 'b';
+
+      export { b };
+      ",
+        "c.cjs": "var a_cjs = require('./a.cjs');
+
+      const c = \`c\${a_cjs.a}\`;
+
+      exports.c = c;
+      ",
+      }
+    `)
+  })
+})
diff --git a/test/integration/default-default-export-different-ext/package.json b/test/integration/default-default-export-different-ext/package.json
@@ -0,0 +1,32 @@
+{
+  "name": "default-default-esm",
+  "type": "module",
+  "exports": {
+    "./a": {
+      "import": {
+        "types": "./dist/a.d.ts",
+        "default": "./dist/a.js"
+      },
+      "require": {
+        "types": "./dist/a.d.cts",
+        "default": "./dist/a.cjs"
+      },
+      "default": "./dist/a.cjs"
+    },
+    "./b": {
+      "import": {
+        "types": "./dist/b.d.ts",
+        "default": "./dist/b.js"
+      },
+      "require": {
+        "types": "./dist/b.d.cts",
+        "default": "./dist/b.cjs"
+      },
+      "default": "./dist/b.cjs"
+    },
+    "./c": {
+      "default": "./dist/c.cjs"
+    }
+  },
+  "private": true
+}
diff --git a/test/integration/default-default-export-different-ext/src/a.ts b/test/integration/default-default-export-different-ext/src/a.ts
@@ -0,0 +1,2 @@
+export * from './b'
+export const a = 'a'
diff --git a/test/integration/default-default-export-different-ext/src/b.ts b/test/integration/default-default-export-different-ext/src/b.ts
@@ -0,0 +1 @@
+export const b = 'b'
diff --git a/test/integration/default-default-export-different-ext/src/c.ts b/test/integration/default-default-export-different-ext/src/c.ts
@@ -0,0 +1,3 @@
+import { a } from './a'
+
+export const c = `c${a}`
diff --git a/test/integration/default-default-export/index.test.ts b/test/integration/default-default-export/index.test.ts
@@ -0,0 +1,54 @@
+import { describe, expect, it } from 'vitest'
+import {
+  getFileNamesFromDirectory,
+  createJob,
+  getFileContents,
+} from '../../testing-utils'
+
+describe('integration - default-default-export', () => {
+  const { distDir } = createJob({ directory: __dirname })
+
+  it('should not export esm module in cjs file', async () => {
+    const distFiles = await getFileNamesFromDirectory(distDir)
+    expect(distFiles).toEqual([
+      'a.cjs',
+      'a.d.ts',
+      'a.js',
+      'b.cjs',
+      'b.d.ts',
+      'b.js',
+      'c.js',
+    ])
+    const contents = await getFileContents(distDir)
+    expect(contents['a.cjs']).toMatchInlineSnapshot(`
+      "var b_cjs = require('./b.cjs');
+
+      const a = 'a';
+
+      exports.a = a;
+      Object.keys(b_cjs).forEach(function (k) {
+      	if (k !== 'default' && !Object.prototype.hasOwnProperty.call(exports, k)) Object.defineProperty(exports, k, {
+      		enumerable: true,
+      		get: function () { return b_cjs[k]; }
+      	});
+      });
+      "
+    `)
+    expect(contents['a.js']).toMatchInlineSnapshot(`
+      "export * from './b.js';
+
+      const a = 'a';
+
+      export { a };
+      "
+    `)
+    expect(contents['c.js']).toMatchInlineSnapshot(`
+      "import { a } from './a.js';
+
+      const c = \`c\${a}\`;
+
+      export { c };
+      "
+    `)
+  })
+})
diff --git a/test/integration/default-default-export/package.json b/test/integration/default-default-export/package.json
@@ -0,0 +1,32 @@
+{
+  "name": "default-default-esm",
+  "type": "module",
+  "exports": {
+    "./a": {
+      "import": {
+        "types": "./dist/a.d.ts",
+        "default": "./dist/a.js"
+      },
+      "require": {
+        "types": "./dist/a.d.cts",
+        "default": "./dist/a.cjs"
+      },
+      "default": "./dist/a.js"
+    },
+    "./b": {
+      "import": {
+        "types": "./dist/b.d.ts",
+        "default": "./dist/b.js"
+      },
+      "require": {
+        "types": "./dist/b.d.cts",
+        "default": "./dist/b.cjs"
+      },
+      "default": "./dist/b.js"
+    },
+    "./c": {
+      "default": "./dist/c.js"
+    }
+  },
+  "private": true
+}
diff --git a/test/integration/default-default-export/src/a.ts b/test/integration/default-default-export/src/a.ts
@@ -0,0 +1,2 @@
+export * from './b'
+export const a = 'a'
diff --git a/test/integration/default-default-export/src/b.ts b/test/integration/default-default-export/src/b.ts
@@ -0,0 +1 @@
+export const b = 'b'
diff --git a/test/integration/default-default-export/src/c.ts b/test/integration/default-default-export/src/c.ts
@@ -0,0 +1,3 @@
+import { a } from './a'
+
+export const c = `c${a}`
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		import { a } from './a'

		export const c = `c${a}`