Repository: electron/electron (language: cpp)

Issue:
[Feature Request]: Allow commonjs2 scripts as sandboxed preload scripts
### Preflight Checklist

- [X] I have read the [Contributing Guidelines](https://github.com/electron/electron/blob/main/CONTRIBUTING.md) for this project.
- [X] I agree to follow the [Code of Conduct](https://github.com/electron/electron/blob/main/CODE_OF_CONDUCT.md) that this project adheres to.
- [X] I have searched the [issue tracker](https://www.github.com/electron/electron/issues) for a feature request that matches the one I want to file, without success.

### Problem Description

When calling a preload script in sandboxed mode, `module` is not defined. This makes code that assigns `module.exports` crash with an error like "ReferenceError: module is not defined". This is made even more confusing since electron also emits an error "Unable to load preload script" which implies that the script was not found, and not just partly executed.

This is a problem because many commonjs modules assign the `module.exports` property, as per [Node's documentation](https://nodejs.org/api/modules.html#the-module-object):

> The `module.exports` object is created by the `Module` system. Sometimes this is not acceptable; many want their module to be an instance of some class. To do this, assign the desired export object to `module.exports`.

Problematic commonjs scripts are created by:
- `webpack` in ['commonjs2' mode](https://webpack.js.org/configuration/output/#type-commonjs2), which is the recommended format (see, e.g. Quasar https://github.com/quasarframework/quasar/issues/11759)
- `esbuild`
- `parcel`

It seems that `rollup`, `swc`, and `typescript` do not reassign `module.exports` and so are not affected.

### Proposed Solution

- Pass a `module` object with a mutable `exports` property to the [preload wrapper](https://github.com/electron/electron/blob/a8c0ed890f61664156886356ecb6ab9f73c69cf1/lib/sandboxed_renderer/init.ts#L105-L107).

### Alternatives Considered

- Load the preload script as an ESM module
- Use a module loader (e.g. an actual call to `require`) instead of interpolating the preload script into an `eval` call.

### Additional Information

I'm not sure if this should be regarded as a feature request or a bug. It's quite surprising behavior and probably not intentional, so I'm leaning toward bug.


Discussion / hints:

Fix patch:
diff --git a/lib/sandboxed_renderer/init.ts b/lib/sandboxed_renderer/init.ts
index cf16e33344fa7..fb3768b62ac51 100644
--- a/lib/sandboxed_renderer/init.ts
+++ b/lib/sandboxed_renderer/init.ts
@@ -120,15 +120,16 @@ require('@electron/internal/renderer/common-init');
 // - `Buffer`: Shim of `Buffer` implementation
 // - `global`: The window object, which is aliased to `global` by webpack.
 function runPreloadScript (preloadSrc: string) {
-  const preloadWrapperSrc = `(function(require, process, Buffer, global, setImmediate, clearImmediate, exports) {
+  const preloadWrapperSrc = `(function(require, process, Buffer, global, setImmediate, clearImmediate, exports, module) {
   ${preloadSrc}
   })`;
 
   // eval in window scope
   const preloadFn = binding.createPreloadScript(preloadWrapperSrc);
   const { setImmediate, clearImmediate } = require('timers');
+  const exports = {};
 
-  preloadFn(preloadRequire, preloadProcess, Buffer, global, setImmediate, clearImmediate, {});
+  preloadFn(preloadRequire, preloadProcess, Buffer, global, setImmediate, clearImmediate, exports, { exports });
 }
 
 for (const { preloadPath, preloadSrc, preloadError } of preloadScripts) {
diff --git a/spec/chromium-spec.ts b/spec/chromium-spec.ts
index f9ebb1f4efa04..771f9e0b50dff 100644
--- a/spec/chromium-spec.ts
+++ b/spec/chromium-spec.ts
@@ -1186,7 +1186,7 @@ describe('chromium features', () => {
       w.webContents.executeJavaScript('window.child = window.open(); child.opener = null');
       const [, { webContents }] = await once(app, 'browser-window-created');
       const [,, message] = await once(webContents, 'console-message');
-      expect(message).to.equal('{"require":"function","module":"undefined","process":"object","Buffer":"function"}');
+      expect(message).to.equal('{"require":"function","module":"object","exports":"object","process":"object","Buffer":"function"}');
     });
 
     it('disables the <webview> tag when it is disabled on the parent window', async () => {
diff --git a/spec/fixtures/module/preload.js b/spec/fixtures/module/preload.js
index aa7bba4cae8f0..3db67d9bde9b1 100644
--- a/spec/fixtures/module/preload.js
+++ b/spec/fixtures/module/preload.js
@@ -1,6 +1,7 @@
 const types = {
   require: typeof require,
   module: typeof module,
+  exports: typeof exports,
   process: typeof process,
   Buffer: typeof Buffer
 };


Test patch: