CommonJS plugin strictRequires settings always hoists NodeJS conditional require calls from an imported UMD file

[Bloodsucker]

Hello everyone,

Given a entry-point index.js:

import { doSomething } from "./umd.cjs";

export function entryPoint() {
    doSomething(false);
}

And a UMD bundle ( umd.cjs ) with "dynamic" require calls to NodeJS builtins that are called only under certain conditions:

// Third-party library UMD library are bundled into this file.
(function (global, factory) {
    typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) :
    typeof define === 'function' && define.amd ? define(['exports'], factory) :
    (global = typeof globalThis !== 'undefined' ? globalThis : global || self, factory(global.umd_bundle = {}));
})(this, (function (exports) { 'use strict';
    function doSomething(isNode) {
        if (isNode) {
            return require('os'); // This require is called "dynamically".
        }
    }
    exports.doSomething = doSomething;
}));

A rollup config rollup.config.js:

export default defineConfig([
	{
		input: 'src/index2.js',
		output: [
			{
				file: 'dist/main_bundle.js',
				format: 'umd',
				name: 'mainBundle',
			},
		],
		plugins: [commonjs(), nodeResolve()],
	},
]);

This results in the following final UMD bundle file:

(function (global, factory) {
	typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('os')) :
	typeof define === 'function' && define.amd ? define(['exports', 'os'], factory) :
	(global = typeof globalThis !== 'undefined' ? globalThis : global || self, factory(global.mainBundle = {}, global.require$$0));
})(this, (function (exports, require$$0) { 'use strict';

	var commonjsGlobal = typeof globalThis !== 'undefined' ? globalThis : typeof window !== 'undefined' ? window : typeof global !== 'undefined' ? global : typeof self !== 'undefined' ? self : {};

	var umd = {exports: {}};

	(function (module, exports) {
		// Third-party library UMD library are bundled into this file.
		(function (global, factory) {
		    factory(exports) ;
		})(commonjsGlobal, (function (exports) {	    function doSomething(isNode) {
		        if (isNode) {
		            return require$$0; // This require is called "dynamically".
		        }
		    }
		    exports.doSomething = doSomething;
		}));
	} (umd, umd.exports));

	function entryPoint() {
	    umd.exports.doSomething(false);
	}

	exports.entryPoint = entryPoint;

}));

The umd.cjs require require('os'); has been replaced with a return require$$0; which changes the logic that is happening inside the bundle. This also causes the following errors:

src/index.js → dist/main_bundle.js...
(!) Missing shims for Node.js built-ins
Creating a browser bundle that depends on "os". You might need to include https://github.com/FredKSchott/rollup-plugin-polyfill-node
(!) Missing global variable name
https://rollupjs.org/guide/en/#outputglobals
Use "output.globals" to specify browser global variable names corresponding to external modules:
os (guessing "require$$0")

I believe this is somehow expected. However, if I use the setting strictRequires from the plugin @rollup/plugin-commonjs in the rollup.config.js:

plugins: [commonjs({strictRequires: true}), nodeResolve()],

The expectation was to NOT hoist the require('os') call in the resulting final UMD file. In other words. From:

(function (global, factory) {
	typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('os')) : // problem!
	typeof define === 'function' && define.amd ? define(['exports', 'os'], factory) :
	(global = typeof globalThis !== 'undefined' ? globalThis : global || self, factory(global.mainBundle = {}, global.require$$0));
})(this, (function (exports, require$$0) { 'use strict';

// ...

to something like (modified manually):

(function (global, factory) {
	typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports) :
	typeof define === 'function' && define.amd ? define(['exports', 'os'], factory) :
	(global = typeof globalThis !== 'undefined' ? globalThis : global || self, factory(global.mainBundle = {}));
})(this, (function (exports) { 'use strict';
// ...

Paradoxically, an imported ES6 module file with dynamic require doesn't result in a hoisted require call.

Does anybody know how to avoid the hoisting of the file and how to fix the bundling errors?

[Bloodsucker]

For anyone interested, the workaround I've is to replace require('os')in the library withglobal.require('os')`. This solves the problem with Rollup. However, this in my opinion an "optimization" that the Rollup plugin should stop doing because it breaks logic.

[xlith]

If I understand correctly, somehow commonjs plugin doesn't consider the require as a dynamic unless it's been wrapped by another method.

Using the following basic setup, combining with the ignoreDynamicRequires option, the hoisting and dynamic import problems can be fixed. But the behavior seems to be a bug.

// rollup.config.js

const { defineConfig } = require("rollup");
const commonjs = require("@rollup/plugin-commonjs");

module.exports = [
  defineConfig({
    input: "index.js",
    context: this,
    output: [
      {
        name: "rollup-dynamic-require",
        file: `output.js`,
        sourcemap: "inline",
        format: "umd",
      },
    ],
    plugins: [
      commonjs({
        ignoreDynamicRequires: true, // this is the option for dynamic imports.
      }),
    ],
  }),
];

// index.js

const dynamicRequire = function (module) {
  return require(module);
};

(function rollupDynamicRequire() {
  let required;
  for (const element of [0, 1, 2]) {
    if (element === 0) {
      required = dynamicRequire("http");
    } else if (element === 1) {
      required = dynamicRequire("https");
    } else if (element === 2) {
      required = dynamicRequire("os");
    }
    console.log(required.globalAgent.protocol);
  }
})();

// output.js

(function (global, factory) {
    typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory() :
    typeof define === 'function' && define.amd ? define(factory) :
    (global = typeof globalThis !== 'undefined' ? globalThis : global || self, global["rollup-dynamic-require"] = factory());
})(this, (function () { 'use strict';

    function getDefaultExportFromCjs (x) {
        return x && x.__esModule && Object.prototype.hasOwnProperty.call(x, 'default') ? x['default'] : x;
    }

    var rollupDynamicRequire = {};

    var hasRequiredRollupDynamicRequire;

    function requireRollupDynamicRequire () {
        if (hasRequiredRollupDynamicRequire) return rollupDynamicRequire;
        hasRequiredRollupDynamicRequire = 1;
        const dynamicRequire = function (module) {
          return require(module);
        };

        (function rollupDynamicRequire() {
          let required;
          for (const element of [0, 1, 2]) {
            if (element === 0) {
              required = dynamicRequire("http");
            } else if (element === 1) {
              required = dynamicRequire("https");
            } else if (element === 2) {
              required = dynamicRequire("os");
            }
            console.log(required.globalAgent.protocol);
          }
        })();
        return rollupDynamicRequire;
    }

    var rollupDynamicRequireExports = requireRollupDynamicRequire();
    var index = /*@__PURE__*/getDefaultExportFromCjs(rollupDynamicRequireExports);

    return index;

}));

[potatohoney-p]

For this particular UMD case, you can leave the library source unchanged and tell the CommonJS plugin not to convert that specific call:

plugins: [
  commonjs({
    strictRequires: true,
    ignore: ['os'],
  }),
  nodeResolve(),
],

ignore takes module IDs whose require calls should remain unconverted. That keeps require('os') inside if (isNode) instead of making os a dependency of the generated UMD wrapper. Include 'node:os' too if the input uses that spelling.

I reproduced this with Rollup 4.60.0, @rollup/plugin-commonjs 29.0.3, and @rollup/plugin-node-resolve 16.0.3, using the UMD source from the question. With strictRequires: true alone, evaluating the bundle's CommonJS branch immediately called require('os'). Adding ignore: ['os'] gave these results:

• No os call while loading the bundle or calling entryPoint().
• Calling the exported doSomething(true) invoked require('os') once, at that point.
• No missing-Node-shim or missing-global warnings for this fixture.
• The AMD wrapper depended only on exports, not os.

The distinction is that strictRequires wraps bundled CommonJS modules; it does not, by itself, stop this external built-in dependency from being hoisted. Also, a literal require('os') is statically identifiable even though execution is conditional, so ignoreDynamicRequires is not the option for that call.

This does not polyfill os: in a browser without require, the false branch works, but actually entering the Node-only branch still throws. It is appropriate only if that branch really stays Node-only. For a Node-only ESM output, the separate requireNodeBuiltins option is worth considering instead; it introduces a node:module dependency, so it is not a browser-UMD replacement.