Silence CJSOnlyExportsDefault in node10/node16-cjs

Author: andrewbranch

packages/cli/package.json

@@ -33,8 +33,7 @@
     "tsc": "tsc -b",
     "local:install": "npm install -g .",
     "local:uninstall": "npm uninstall -g @arethetypeswrong/cli",
-    "pretest": "tsc -b test",
-    "test": "node --test test/dist",
+    "test": "tsc -b test && node --test test/dist",
     "prepack": "pnpm tsc"
   },
   "type": "module",

packages/cli/test/snapshots/[email protected]

@@ -12,11 +12,11 @@ Build tools:
 🤨 CommonJS module simulates a default export with exports.default and exports.__esModule, but does not also set module.exports for compatibility with Node. Node, and some bundlers under certain conditions (https://andrewbranch.github.io/interop-test/#synthesizing-default-exports-for-cjs-modules), do not respect the __esModule marker, so accessing the intended default export will require a .default property access on the default import. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/CJSOnlyExportsDefault.md
 
 
-┌────────────────────┬───────────────────────┬───────────────────────┬───────────────────────┬───────────────────────┐
-│                    │ node10                │ node16 (from CJS)     │ node16 (from ESM)     │ bundler               │
-├────────────────────┼───────────────────────┼───────────────────────┼───────────────────────┼───────────────────────┤
-│ "node-html-parser" │ 🤨 CJS default export │ 🤨 CJS default export │ 🤨 CJS default export │ 🤨 CJS default export │
-└────────────────────┴───────────────────────┴───────────────────────┴───────────────────────┴───────────────────────┘
+┌────────────────────┬────────┬───────────────────┬───────────────────────┬───────────────────────┐
+│                    │ node10 │ node16 (from CJS) │ node16 (from ESM)     │ bundler               │
+├────────────────────┼────────┼───────────────────┼───────────────────────┼───────────────────────┤
+│ "node-html-parser" │ 🟢     │ 🟢 (CJS)          │ 🤨 CJS default export │ 🤨 CJS default export │
+└────────────────────┴────────┴───────────────────┴───────────────────────┴───────────────────────┘
 
 
 ```

packages/core/package.json

@@ -19,8 +19,7 @@
   },
   "scripts": {
     "tsc": "tsc",
-    "pretest": "tsc -b test",
-    "test": "node --test test/dist",
+    "test": "tsc -b test && node --test test/dist",
     "snapshot": "node scripts/createSnapshotFixture.js",
     "prepack": "pnpm tsc"
   },

packages/core/src/internal/checks/cjsOnlyExportsDefault.ts

@@ -6,12 +6,31 @@ export default defineCheck({
   dependencies: ({ entrypoints, subpath, resolutionKind }) => {
     const entrypoint = entrypoints[subpath].resolutions[resolutionKind];
     const implementationFileName = entrypoint.implementationResolution?.fileName;
-    return [implementationFileName];
+    return [implementationFileName, resolutionKind];
   },
-  execute: ([implementationFileName], context) => {
+  execute: ([implementationFileName, resolutionKind], context) => {
     if (!implementationFileName) {
       return;
     }
+    if (resolutionKind === "node10" || resolutionKind === "node16-cjs") {
+      // Here, we have a CJS file (most likely transpiled ESM) resolving to a
+      // CJS transpiled ESM file. This is fine when considered in isolation.
+      // The pattern of having `module.exports.default = ...` is a problem
+      // primarily because ESM-detected files in Node (and the same files in
+      // Webpack/esbuild) will treat `module.exports` as the default export,
+      // which is both unexpected and different from Babel-style interop seen
+      // in transpiled default imports and most bundler scenarios. But if Node,
+      // Webpack, and esbuild never see this file, then it's fine. So, while
+      // the problematic pattern is a feature of the file alone, the bad outcome
+      // comes from a combination of the file and the module system that imports
+      // it. For dual packages that point Node imports and bundlers to a true
+      // ESM default export, while pointing requires to this CJS "default export,"
+      // we don't want to report a problem.
+      //
+      // TODO: It would be nice to report this information *somehow*, as neutral
+      // metadata attached to the file (c.f. `Analysis["programInfo"]`).
+      return;
+    }
     const host = context.hosts.findHostForFiles([implementationFileName]) ?? context.hosts.bundler;
     const sourceFile = host.getSourceFile(implementationFileName)!;
     if (

packages/core/test/snapshots/[email protected]

@@ -81,9 +81,7 @@
             "/node_modules/node-html-parser/dist/valid.d.ts",
             "/node_modules/node-html-parser/dist/index.d.ts"
           ],
-          "visibleProblems": [
-            0
-          ]
+          "visibleProblems": []
         },
         "node16-cjs": {
           "name": ".",
@@ -158,9 +156,7 @@
             "/node_modules/node-html-parser/dist/valid.d.ts",
             "/node_modules/node-html-parser/dist/index.d.ts"
           ],
-          "visibleProblems": [
-            0
-          ]
+          "visibleProblems": []
         },
         "node16-esm": {
           "name": ".",
@@ -342,7 +338,7 @@
             "/node_modules/node-html-parser/dist/index.d.ts"
           ],
           "visibleProblems": [
-            0
+            1
           ]
         }
       },
@@ -414,6 +410,12 @@
     "bundler": {}
   },
   "problems": [
+    {
+      "kind": "CJSOnlyExportsDefault",
+      "fileName": "/node_modules/node-html-parser/dist/index.js",
+      "pos": 1077,
+      "end": 1092
+    },
     {
       "kind": "CJSOnlyExportsDefault",
       "fileName": "/node_modules/node-html-parser/dist/index.js",

packages/history/package.json

@@ -29,8 +29,7 @@
   "scripts": {
     "tsc": "tsc -b",
     "build:scripts": "tsc -b scripts",
-    "pregenerate": "pnpm build:scripts",
-    "generate": "tsx scripts/generateFull.ts",
+    "generate": "pnpm build:scripts && tsx scripts/generateFull.ts",
     "prepublishOnly": "pnpm tsc && pnpm generate"
   },
   "dependencies": {