fix(commonjs): treat moduleSideEffects as __PURE__ comments for proxi…

…ed wrapped modules
rollup · Jan 19, 2024 · 3d78403 · 3d78403
1 parent ccdf1c5
commit 3d78403
Show file tree

Hide file tree

Showing 8 changed files with 63 additions and 7 deletions.
diff --git a/packages/commonjs/src/index.js b/packages/commonjs/src/index.js
@@ -1,4 +1,4 @@
-import { extname, relative, resolve, dirname } from 'path';
+import { dirname, extname, relative, resolve } from 'path';
 
 import { createFilter } from '@rollup/pluginutils';
 
@@ -239,7 +239,7 @@ export default function commonjs(options = {}) {
       }
     },
 
-    load(id) {
+    async load(id) {
       if (id === HELPERS_ID) {
         return getHelpersModule();
       }
@@ -285,7 +285,11 @@ export default function commonjs(options = {}) {
 
       if (isWrappedId(id, ES_IMPORT_SUFFIX)) {
         const actualId = unwrapId(id, ES_IMPORT_SUFFIX);
-        return getEsImportProxy(actualId, getDefaultIsModuleExports(actualId));
+        return getEsImportProxy(
+          actualId,
+          getDefaultIsModuleExports(actualId),
+          (await this.load({ id: actualId })).moduleSideEffects
+        );
       }
 
       if (id === DYNAMIC_MODULES_ID) {

diff --git a/packages/commonjs/src/proxies.js b/packages/commonjs/src/proxies.js
@@ -57,21 +57,21 @@ export function getEntryProxy(id, defaultIsModuleExports, getModuleInfo, shebang
     }
     return shebang + code;
   }
-  const result = getEsImportProxy(id, defaultIsModuleExports);
+  const result = getEsImportProxy(id, defaultIsModuleExports, true);
   return {
     ...result,
     code: shebang + result.code
   };
 }
 
-export function getEsImportProxy(id, defaultIsModuleExports) {
+export function getEsImportProxy(id, defaultIsModuleExports, moduleSideEffects) {
   const name = getName(id);
   const exportsName = `${name}Exports`;
   const requireModule = `require${capitalize(name)}`;
   let code =
     `import { getDefaultExportFromCjs } from "${HELPERS_ID}";\n` +
     `import { __require as ${requireModule} } from ${JSON.stringify(id)};\n` +
-    `var ${exportsName} = ${requireModule}();\n` +
+    `var ${exportsName} = ${moduleSideEffects ? '' : '/*@__PURE__*/ '}${requireModule}();\n` +
     `export { ${exportsName} as __moduleExports };`;
   if (defaultIsModuleExports === true) {
     code += `\nexport { ${exportsName} as default };`;

diff --git a/packages/commonjs/test/fixtures/function/module-side-effects-import-wrapped/_config.js b/packages/commonjs/test/fixtures/function/module-side-effects-import-wrapped/_config.js
@@ -0,0 +1,24 @@
+module.exports = {
+  description: 'respects module-side-effects when importing wrapped dependencies',
+  options: {
+    plugins: [
+      {
+        name: 'test',
+        async resolveId(source, importer, options) {
+          if (source.endsWith('./foo.js')) {
+            const resolved = await this.resolve(source, importer, options);
+            return { ...resolved, moduleSideEffects: false };
+          }
+          return null;
+        }
+      }
+    ]
+  },
+  pluginOptions: {
+    strictRequires: true
+  },
+  global: (global, t) => {
+    t.is(global.foo, undefined);
+    t.is(global.bar, 'bar');
+  }
+};
diff --git a/packages/commonjs/test/fixtures/function/module-side-effects-import-wrapped/bar.js b/packages/commonjs/test/fixtures/function/module-side-effects-import-wrapped/bar.js
@@ -0,0 +1 @@
+global.bar = 'bar';
diff --git a/packages/commonjs/test/fixtures/function/module-side-effects-import-wrapped/foo.js b/packages/commonjs/test/fixtures/function/module-side-effects-import-wrapped/foo.js
@@ -0,0 +1 @@
+global.foo = 'foo';
diff --git a/packages/commonjs/test/fixtures/function/module-side-effects-import-wrapped/main.js b/packages/commonjs/test/fixtures/function/module-side-effects-import-wrapped/main.js
@@ -0,0 +1,2 @@
+import { foo } from './foo.js';
+import { bar } from './bar.js';
diff --git a/packages/commonjs/test/snapshots/function.js.md b/packages/commonjs/test/snapshots/function.js.md
@@ -6545,6 +6545,30 @@ Generated by [AVA](https://avajs.dev).
       `,
     }
 
+## module-side-effects-import-wrapped
+
+> Snapshot 1
+
+    {
+      'main.js': `'use strict';␊
+      ␊
+      var commonjsGlobal = typeof globalThis !== 'undefined' ? globalThis : typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : {};␊
+      ␊
+      var bar = {};␊
+      ␊
+      var hasRequiredBar;␊
+      ␊
+      function requireBar () {␊
+      	if (hasRequiredBar) return bar;␊
+      	hasRequiredBar = 1;␊
+      	commonjsGlobal.bar = 'bar';␊
+      	return bar;␊
+      }␊
+      ␊
+      requireBar();␊
+      `,
+    }
+
 ## module-side-effects-late-entry
 
 > Snapshot 1
@@ -6613,7 +6637,7 @@ Generated by [AVA](https://avajs.dev).
       `,
     }
 
-## module-side-effects-wrapped
+## module-side-effects-require-wrapped
 
 > Snapshot 1
 

diff --git a/packages/commonjs/test/snapshots/function.js.snap b/packages/commonjs/test/snapshots/function.js.snap
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		import { foo } from './foo.js';
		import { bar } from './bar.js';