Consider moduleSideEffects as __PURE__ comments for wrapped modules

rollup · Jan 19, 2024 · 4e966ea · 4e966ea
1 parent 95bb728
commit 4e966ea
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 6 deletions.
diff --git a/packages/commonjs/src/generate-imports.js b/packages/commonjs/src/generate-imports.js
@@ -165,7 +165,7 @@ function processRequireExpressions(
   magicString
 ) {
   const generateRequireName = getGenerateRequireName();
-  for (const { source, id: resolvedId, isCommonJS } of requireTargets) {
+  for (const { source, id: resolvedId, isCommonJS, wrappedModuleSideEffects } of requireTargets) {
     const requires = requiresBySource[source];
     const name = generateRequireName(requires);
     let usesRequired = false;
@@ -184,7 +184,11 @@ function processRequireExpressions(
       } else if (canConvertRequire) {
         needsImport = true;
         if (isCommonJS === IS_WRAPPED_COMMONJS) {
-          magicString.overwrite(node.start, node.end, `${name}()`);
+          magicString.overwrite(
+            node.start,
+            node.end,
+            `${wrappedModuleSideEffects ? '' : '/*@__PURE__*/ '}${name}()`
+          );
         } else if (usesReturnValue) {
           usesRequired = true;
           magicString.overwrite(node.start, node.end, name);

diff --git a/packages/commonjs/src/resolve-require-sources.js b/packages/commonjs/src/resolve-require-sources.js
@@ -192,13 +192,14 @@ export function getRequireResolver(extensions, detectCyclesAndConditional, curre
           // eslint-disable-next-line no-multi-assign
           const isCommonJS = (parentMeta.isRequiredCommonJS[dependencyId] =
             getTypeForFullyAnalyzedModule(dependencyId));
+          const isWrappedCommonJS = isCommonJS === IS_WRAPPED_COMMONJS;
           fullyAnalyzedModules[dependencyId] = true;
           return {
+            wrappedModuleSideEffects:
+              isWrappedCommonJS && rollupContext.getModuleInfo(dependencyId).moduleSideEffects,
             source: sources[index].source,
             id: allowProxy
-              ? isCommonJS === IS_WRAPPED_COMMONJS
-                ? wrapId(dependencyId, WRAPPED_SUFFIX)
-                : wrapId(dependencyId, PROXY_SUFFIX)
+              ? wrapId(dependencyId, isWrappedCommonJS ? WRAPPED_SUFFIX : PROXY_SUFFIX)
               : dependencyId,
             isCommonJS
           };

diff --git a/packages/commonjs/test/fixtures/function/module-side-effects-require-wrapped/_config.js b/packages/commonjs/test/fixtures/function/module-side-effects-require-wrapped/_config.js
@@ -0,0 +1,24 @@
+module.exports = {
+  description: 'respects module-side-effects when requiring 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-require-wrapped/bar.js b/packages/commonjs/test/fixtures/function/module-side-effects-require-wrapped/bar.js
@@ -0,0 +1 @@
+global.bar = 'bar';
diff --git a/packages/commonjs/test/fixtures/function/module-side-effects-require-wrapped/foo.js b/packages/commonjs/test/fixtures/function/module-side-effects-require-wrapped/foo.js
@@ -0,0 +1 @@
+global.foo = 'foo';
diff --git a/packages/commonjs/test/fixtures/function/module-side-effects-require-wrapped/main.js b/packages/commonjs/test/fixtures/function/module-side-effects-require-wrapped/main.js
@@ -0,0 +1,2 @@
+const unusedFoo = require('./foo.js');
+const unusedBar = require('./bar.js');
diff --git a/packages/commonjs/test/snapshots/function.js.md b/packages/commonjs/test/snapshots/function.js.md
@@ -6613,6 +6613,48 @@ Generated by [AVA](https://avajs.dev).
       `,
     }
 
+## module-side-effects-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 : {};␊
+      ␊
+      function getDefaultExportFromCjs (x) {␊
+      	return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, 'default') ? x['default'] : x;␊
+      }␊
+      ␊
+      var main$1 = {};␊
+      ␊
+      var bar = {};␊
+      ␊
+      var hasRequiredBar;␊
+      ␊
+      function requireBar () {␊
+      	if (hasRequiredBar) return bar;␊
+      	hasRequiredBar = 1;␊
+      	commonjsGlobal.bar = 'bar';␊
+      	return bar;␊
+      }␊
+      ␊
+      var hasRequiredMain;␊
+      ␊
+      function requireMain () {␊
+      	if (hasRequiredMain) return main$1;␊
+      	hasRequiredMain = 1;␊
+      	requireBar();␊
+      	return main$1;␊
+      }␊
+      ␊
+      var mainExports = requireMain();␊
+      var main = /*@__PURE__*/getDefaultExportFromCjs(mainExports);␊
+      ␊
+      module.exports = main;␊
+      `,
+    }
+
 ## module_require
 
 > Snapshot 1
@@ -7106,7 +7148,7 @@ Generated by [AVA](https://avajs.dev).
       function requireMain () {␊
       	if (hasRequiredMain) return main$1;␊
       	hasRequiredMain = 1;␊
-      	const dep = requireDep();␊
+      	const dep = /*@__PURE__*/ requireDep();␊
       ␊
       	t.is(dep.foo, 'bar');␊
       	return main$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 @@
		const unusedFoo = require('./foo.js');
		const unusedBar = require('./bar.js');