fix(commonjs): Do not change semantics when removing requires in if statements (#1038)

Author: lukastaegert

packages/commonjs/src/generate-imports.js

@@ -60,7 +60,7 @@ export function getRequireStringArg(node) {
 export function getRequireHandlers() {
   const requireExpressions = [];
 
-  function addRequireStatement(
+  function addRequireExpression(
     sourceId,
     node,
     scope,
@@ -140,7 +140,7 @@ export function getRequireHandlers() {
   }
 
   return {
-    addRequireStatement,
+    addRequireExpression,
     rewriteRequireExpressionsAndGetImportBlock
   };
 }

packages/commonjs/src/transform-commonjs.js

@@ -78,7 +78,7 @@ export default async function transformCommonjs(
   // unconditional require elsewhere.
   let currentConditionalNodeEnd = null;
   const conditionalNodes = new Set();
-  const { addRequireStatement, rewriteRequireExpressionsAndGetImportBlock } = getRequireHandlers();
+  const { addRequireExpression, rewriteRequireExpressionsAndGetImportBlock } = getRequireHandlers();
 
   // See which names are assigned to. This is necessary to prevent
   // illegally replacing `var foo = require('foo')` with `import foo from 'foo'`,
@@ -233,14 +233,22 @@ export default async function transformCommonjs(
           const requireStringArg = getRequireStringArg(node);
           if (!ignoreRequire(requireStringArg)) {
             const usesReturnValue = parent.type !== 'ExpressionStatement';
-            addRequireStatement(
+            const toBeRemoved =
+              parent.type === 'ExpressionStatement' &&
+              (!currentConditionalNodeEnd ||
+                // We should completely remove requires directly in a try-catch
+                // so that Rollup can remove up the try-catch
+                (currentTryBlockEnd !== null && currentTryBlockEnd < currentConditionalNodeEnd))
+                ? parent
+                : node;
+            addRequireExpression(
               requireStringArg,
               node,
               scope,
               usesReturnValue,
               currentTryBlockEnd !== null,
               currentConditionalNodeEnd !== null,
-              parent.type === 'ExpressionStatement' ? parent : node
+              toBeRemoved
             );
             if (parent.type === 'VariableDeclarator' && parent.id.type === 'Identifier') {
               for (const name of extractAssignedNames(parent.id)) {
@@ -442,6 +450,7 @@ export default async function transformCommonjs(
     !(
       shouldWrap ||
       isRequired ||
+      needsRequireWrapper ||
       uses.module ||
       uses.exports ||
       uses.require ||

packages/commonjs/test/fixtures/function/conditional-require-non-strict/_config.js

@@ -0,0 +1,4 @@
+module.exports = {
+  description: 'handles conditional require statements in non-strict mode',
+  pluginOptions: { strictRequires: false }
+};

packages/commonjs/test/fixtures/function/conditional-require-non-strict/bar.js

@@ -0,0 +1 @@
+global.bar = true;

packages/commonjs/test/fixtures/function/conditional-require-non-strict/foo.js

@@ -0,0 +1 @@
+global.foo = true;

packages/commonjs/test/fixtures/function/conditional-require-non-strict/main.js

@@ -0,0 +1,7 @@
+if (Math.random() < 2) require('./foo.js');
+if (Math.random() > 2) require('./bar.js');
+
+global.main = true;
+
+t.is(global.foo, true, 'foo');
+t.is(global.main, true, 'main');

packages/commonjs/test/fixtures/function/dynamic-require-empty/_config.js

@@ -0,0 +1,6 @@
+module.exports = {
+  description: 'allows dynamically requiring an empty file',
+  pluginOptions: {
+    dynamicRequireTargets: ['fixtures/function/dynamic-require-empty/submodule.js']
+  }
+};

packages/commonjs/test/fixtures/function/dynamic-require-empty/main.js

@@ -0,0 +1,6 @@
+/* eslint-disable import/no-dynamic-require, global-require */
+function takeModule(withName) {
+  return require(`./${withName}`);
+}
+
+t.deepEqual(takeModule('submodule'), {});

packages/commonjs/test/fixtures/function/dynamic-require-empty/submodule.js

@@ -258,6 +258,30 @@ Generated by [AVA](https://avajs.dev).
       `,
     }
 
+## conditional-require-non-strict
+
+> 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 main = {};␊
+      ␊
+      commonjsGlobal.foo = true;␊
+      ␊
+      commonjsGlobal.bar = true;␊
+      ␊
+      commonjsGlobal.main = true;␊
+      ␊
+      t.is(commonjsGlobal.foo, true, 'foo');␊
+      t.is(commonjsGlobal.main, true, 'main');␊
+      ␊
+      module.exports = main;␊
+      `,
+    }
+
 ## custom-options
 
 > Snapshot 1
@@ -7565,3 +7589,127 @@ Generated by [AVA](https://avajs.dev).
       module.exports = main;␊
       `,
     }
+
+## dynamic-require-empty
+
+> Snapshot 1
+
+    {
+      'main.js': `'use strict';␊
+      ␊
+      var submodule = {};␊
+      ␊
+      var hasRequiredSubmodule;␊
+      ␊
+      function requireSubmodule () {␊
+      	if (hasRequiredSubmodule) return submodule;␊
+      	hasRequiredSubmodule = 1;␊
+      ␊
+      	return submodule;␊
+      }␊
+      ␊
+      var dynamicModules;␊
+      ␊
+      function getDynamicModules() {␊
+      	return dynamicModules || (dynamicModules = {␊
+      		"/fixtures/function/dynamic-require-empty/submodule.js": requireSubmodule␊
+      	});␊
+      }␊
+      ␊
+      function createCommonjsRequire(originalModuleDir) {␊
+      	function handleRequire(path) {␊
+      		var resolvedPath = commonjsResolve(path, originalModuleDir);␊
+      		if (resolvedPath !== null) {␊
+      			return getDynamicModules()[resolvedPath]();␊
+      		}␊
+      		throw new Error('Could not dynamically require "' + path + '". Please configure the dynamicRequireTargets or/and ignoreDynamicRequires option of @rollup/plugin-commonjs appropriately for this require call to work.');␊
+      	}␊
+      	handleRequire.resolve = function (path) {␊
+      		var resolvedPath = commonjsResolve(path, originalModuleDir);␊
+      		if (resolvedPath !== null) {␊
+      			return resolvedPath;␊
+      		}␊
+      		return require.resolve(path);␊
+      	};␊
+      	return handleRequire;␊
+      }␊
+      ␊
+      function commonjsResolve (path, originalModuleDir) {␊
+      	var shouldTryNodeModules = isPossibleNodeModulesPath(path);␊
+      	path = normalize(path);␊
+      	var relPath;␊
+      	if (path[0] === '/') {␊
+      		originalModuleDir = '';␊
+      	}␊
+      	var modules = getDynamicModules();␊
+      	var checkedExtensions = ['', '.js', '.json'];␊
+      	while (true) {␊
+      		if (!shouldTryNodeModules) {␊
+      			relPath = normalize(originalModuleDir + '/' + path);␊
+      		} else {␊
+      			relPath = normalize(originalModuleDir + '/node_modules/' + path);␊
+      		}␊
+      ␊
+      		if (relPath.endsWith('/..')) {␊
+      			break; // Travelled too far up, avoid infinite loop␊
+      		}␊
+      ␊
+      		for (var extensionIndex = 0; extensionIndex < checkedExtensions.length; extensionIndex++) {␊
+      			var resolvedPath = relPath + checkedExtensions[extensionIndex];␊
+      			if (modules[resolvedPath]) {␊
+      				return resolvedPath;␊
+      			}␊
+      		}␊
+      		if (!shouldTryNodeModules) break;␊
+      		var nextDir = normalize(originalModuleDir + '/..');␊
+      		if (nextDir === originalModuleDir) break;␊
+      		originalModuleDir = nextDir;␊
+      	}␊
+      	return null;␊
+      }␊
+      ␊
+      function isPossibleNodeModulesPath (modulePath) {␊
+      	var c0 = modulePath[0];␊
+      	if (c0 === '/' || c0 === '\\\\') return false;␊
+      	var c1 = modulePath[1], c2 = modulePath[2];␊
+      	if ((c0 === '.' && (!c1 || c1 === '/' || c1 === '\\\\')) ||␊
+      		(c0 === '.' && c1 === '.' && (!c2 || c2 === '/' || c2 === '\\\\'))) return false;␊
+      	if (c1 === ':' && (c2 === '/' || c2 === '\\\\')) return false;␊
+      	return true;␊
+      }␊
+      ␊
+      function normalize (path) {␊
+      	path = path.replace(/\\\\/g, '/');␊
+      	var parts = path.split('/');␊
+      	var slashed = parts[0] === '';␊
+      	for (var i = 1; i < parts.length; i++) {␊
+      		if (parts[i] === '.' || parts[i] === '') {␊
+      			parts.splice(i--, 1);␊
+      		}␊
+      	}␊
+      	for (var i = 1; i < parts.length; i++) {␊
+      		if (parts[i] !== '..') continue;␊
+      		if (i > 0 && parts[i - 1] !== '..' && parts[i - 1] !== '.') {␊
+      			parts.splice(--i, 2);␊
+      			i--;␊
+      		}␊
+      	}␊
+      	path = parts.join('/');␊
+      	if (slashed && path[0] !== '/') path = '/' + path;␊
+      	else if (path.length === 0) path = '.';␊
+      	return path;␊
+      }␊
+      ␊
+      var main = {};␊
+      ␊
+      /* eslint-disable import/no-dynamic-require, global-require */␊
+      ␊
+      function takeModule(withName) {␊
+        return createCommonjsRequire("/fixtures/function/dynamic-require-empty")(`./${withName}`);␊
+      }␊
+      ␊
+      t.deepEqual(takeModule('submodule'), {});␊
+      ␊
+      module.exports = main;␊
+      `,
+    }