fix: remove node.module recommendation

sanity-io · Oct 4, 2023 · 1c73247 · 1c73247
1 parent 1295494
commit 1c73247
Show file tree

Hide file tree

Showing 10 changed files with 2 additions and 74 deletions.
diff --git a/playground/default-export/package.json b/playground/default-export/package.json
@@ -9,7 +9,6 @@
       "source": "./src/index.js",
       "require": "./dist/index.cjs",
       "node": {
-        "module": "./dist/index.js",
         "import": "./dist/index.cjs.js"
       },
       "import": "./dist/index.js",

diff --git a/playground/dummy-commonjs/package.json b/playground/dummy-commonjs/package.json
@@ -14,7 +14,6 @@
         "require": "./dist/index.browser.js"
       },
       "node": {
-        "module": "./dist/index.mjs",
         "import": "./node/index.mjs",
         "require": "./node/index.js"
       },
@@ -31,7 +30,6 @@
         "require": "./dist/extra.browser.js"
       },
       "node": {
-        "module": "./dist/extra.mjs",
         "import": "./node/extra.mjs",
         "require": "./node/extra.js"
       },

diff --git a/playground/dummy-module/package.json b/playground/dummy-module/package.json
@@ -14,7 +14,6 @@
         "require": "./dist/index.browser.cjs"
       },
       "node": {
-        "module": "./dist/index.js",
         "import": "./node/index.js",
         "require": "./node/index.cjs"
       },
@@ -31,7 +30,6 @@
         "require": "./dist/extra.browser.cjs"
       },
       "node": {
-        "module": "./dist/extra.js",
         "import": "./node/extra.js",
         "require": "./node/extra.cjs"
       },

diff --git a/playground/multi-export/package.json b/playground/multi-export/package.json
@@ -10,7 +10,6 @@
       "source": "./src/index.ts",
       "require": "./dist/index.cjs",
       "node": {
-        "module": "./dist/index.js",
         "import": "./dist/index.cjs.js"
       },
       "import": "./dist/index.js",
@@ -21,7 +20,6 @@
       "source": "./src/plugin.ts",
       "require": "./dist/plugin.cjs",
       "node": {
-        "module": "./dist/plugin.js",
         "import": "./dist/plugin.cjs.js"
       },
       "import": "./dist/plugin.js",

diff --git a/playground/multi-exports-commonjs/package.json b/playground/multi-exports-commonjs/package.json
@@ -10,7 +10,6 @@
       "source": "./src/index.ts",
       "require": "./dist/index.js",
       "node": {
-        "module": "./dist/index.esm.js",
         "import": "./dist/index.cjs.mjs"
       },
       "import": "./dist/index.esm.js",
@@ -21,7 +20,6 @@
       "source": "./src/plugin.ts",
       "require": "./dist/plugin.js",
       "node": {
-        "module": "./dist/plugin.esm.js",
         "import": "./dist/plugin.cjs.mjs"
       },
       "import": "./dist/plugin.esm.js",

diff --git a/playground/ts-bundler/package.json b/playground/ts-bundler/package.json
@@ -10,7 +10,6 @@
       "source": "./src/index.ts",
       "require": "./dist/index.cjs",
       "node": {
-        "module": "./dist/index.js",
         "import": "./dist/index.cjs.js"
       },
       "import": "./dist/index.js",

diff --git a/playground/ts-node16/package.json b/playground/ts-node16/package.json
@@ -10,7 +10,6 @@
       "source": "./src/index.ts",
       "require": "./dist/index.cjs",
       "node": {
-        "module": "./dist/index.js",
         "import": "./dist/index.cjs.js"
       },
       "import": "./dist/index.js",

diff --git a/playground/ts/package.json b/playground/ts/package.json
@@ -10,7 +10,6 @@
       "source": "./src/index.ts",
       "require": "./dist/index.cjs",
       "node": {
-        "module": "./dist/index.js",
         "import": "./dist/index.cjs.js"
       },
       "import": "./dist/index.js",

diff --git a/src/node/core/pkg/loadPkgWithReporting.ts b/src/node/core/pkg/loadPkgWithReporting.ts
@@ -34,51 +34,17 @@ export async function loadPkgWithReporting(options: {
         }
 
         if (exp.node) {
-          const nodeKeys = Object.keys(exp.node)
-
-          if (!assertOrder('module', 'import', nodeKeys)) {
-            shouldError = true
-            logger.error(
-              `exports["${expPath}"]: the \`node.module\` property should come before the \`node.import\` property`,
-            )
-          }
-
-          if (!assertOrder('import', 'require', nodeKeys)) {
-            logger.warn(
-              `exports["${expPath}"]: the \`node.import\` property should come before the \`node.require\` property`,
-            )
-          }
-
-          if (!assertOrder('module', 'require', nodeKeys)) {
-            logger.warn(
-              `exports["${expPath}"]: the \`node.module\` property should come before \`node.require\` property`,
-            )
-          }
-
           if (exp.import && exp.node.import && !assertOrder('node', 'import', keys)) {
             shouldError = true
             logger.error(
               `exports["${expPath}"]: the \`node\` property should come before the \`import\` property`,
             )
           }
 
-          if (exp.module && exp.node.module && !assertOrder('node', 'module', keys)) {
-            shouldError = true
-            logger.error(
-              `exports["${expPath}"]: the \`node\` property should come before the \`module\` property`,
-            )
-          }
-
-          if (
-            exp.node.import &&
-            !exp.node.require &&
-            exp.node.module &&
-            exp.import &&
-            exp.node.module !== exp.import
-          ) {
+          if (exp.node.module) {
             shouldError = true
             logger.error(
-              `exports["${expPath}"]: the \`node.module\` property should match \`import\``,
+              `exports["${expPath}"]: the \`node.module\` condition shouldn't be used as it's not well supported in all bundlers. A better strategy is to refactor the codebase to no longer be vulnerable to the "dual package hazard"`,
             )
           }
 

diff --git a/test/parseExports.test.ts b/test/parseExports.test.ts
@@ -132,7 +132,6 @@ describe('parseExports', () => {
 
     expect(() => parseExports({extMap, pkg, strict: true})).toThrow(
       '\n- package.json: mismatch between "main" and "exports.require". These must be equal.' +
-        '\n- package.json: mismatch between "module" and "exports.import" These must be equal.' +
         '\n- package.json: `exports["./package.json"] must be "./package.json".' +
         '\n- package.json with `type: "commonjs"` - `exports["."].require` must end with ".js"' +
         '\n- package.json with `type: "commonjs"` - `exports["."].import` must end with ".mjs"',
@@ -292,31 +291,6 @@ describe('parseExports', () => {
         },
         `"node.require" is unnecesary if it's the same as "require"`,
       ],
-      [
-        {
-          source: './src/index.ts',
-          require: './dist/index.cjs',
-          node: {
-            import: './dist/index.cjs.js',
-          },
-          import: './dist/index.js',
-          default: './dist/index.js',
-        },
-        `"node.module" should be specified when a "node.import" dual package hazard is used`,
-      ],
-      [
-        {
-          source: './src/index.ts',
-          require: './dist/index.cjs',
-          module: './dist/index.js',
-          node: {
-            import: './dist/index.cjs.js',
-          },
-          import: './dist/index.js',
-          default: './dist/index.js',
-        },
-        `"module" should be moved to "node.module" when it's the same as "import"`,
-      ],
       [
         {
           source: './src/index.ts',