twolfson / karma-electron

Karma launcher and preprocessor for Electron
The Unlicense
59 stars 21 forks source link

Non-context aware native modules in renderer will cause specs to error #48

Closed EngJay closed 4 years ago

EngJay commented 4 years ago

Due to the breaking change beginning in Electron 9 in which allowRendererProcessReuse is set to true by default, specs that cover modules that use non-context aware native modules, like serialport, in the renderer will fail when using the karma-electron plugin:

Error: Loading non-context-aware native module in renderer: 'PATH_TO_PROJECT\node_modules\@serialport\bindings\build\Release\bindings.node', but app.allowRendererProcessReuse is true. See https://github.com/electron/electron/issues/18397.
    at process.func [as dlopen] (electron/js2c/asar.js:140:31)
    at Object.Module._extensions..node (internal/modules/cjs/loader.js:1034:18)
    at Object.func [as .node] (electron/js2c/asar.js:140:31)
    at Module.load (internal/modules/cjs/loader.js:815:32)
    at Module._load (internal/modules/cjs/loader.js:727:14)
    at Function.Module._load (electron/js2c/asar.js:769:28)
    at Module.require (internal/modules/cjs/loader.js:852:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at bindings PATH_TO_PROJECT\node_modules\bindings\bindings.js:112:48)
    at Object.<anonymous> (PATH_TO_PROJECT\node_modules\@serialport\bindings\lib\win32.js:1:129)
     Error: Cannot instantiate cyclic dependency! DatabaseService
    at throwCyclicDependencyError (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:5438:1)
    at R3Injector.hydrate (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:11344:1)
    at R3Injector.get (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:11170:1)
    at injectInjectorOnly (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:801:1)
    at ɵɵinject (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:805:1)
    at Object.StateService_Factory [as factory] (ng:///StateService/ɵfac.js:5:40)
    at R3Injector.hydrate (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:11348:1)
    at R3Injector.get (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:11170:1)
    at NgModuleRef$1.get (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:23935:1)
    at TestBedRender3.inject (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/testing.js:1930:1)

The transitional solution to this provided by the Electron team is to set allowRendererProcessReuseto false until version 11 of Electron but the option to set allowRendererProcessReuse to false does not appear to be exposed anywhere that can be set via a CLI argument or otherwise externally.

To temporarily fix this, I added

app.allowRendererProcessReuse = false;

to lib/electron-launcher.js and created a patch with patch-package:

diff --git a/node_modules/karma-electron/lib/electron-launcher.js b/node_modules/karma-electron/lib/electron-launcher.js
index 28e779c..ac01a5a 100644
--- a/node_modules/karma-electron/lib/electron-launcher.js
+++ b/node_modules/karma-electron/lib/electron-launcher.js
@@ -35,6 +35,10 @@ app.on('window-all-closed', function handleWindowsClosed () {
   app.quit();
 });

+// Allow non-context aware modules to run in renderer in Electron 9+
+// https://github.com/electron/electron/issues/18397
+app.allowRendererProcessReuse = false;
+
 // Update `userData` before Electron loads
 // DEV: This prevents cookies/localStorage from contaminating across apps
 app.setPath('userData', program.userDataDir);

This will keep our specs working until either any non-context aware dependencies catch-up to Electron's requirements or we move the usage of all of them to Electron's main process.

Is this something that would be appropriate to implement as an option in karma-electron? If so, I can write-up a PR.

twolfson commented 4 years ago

It seems like there's a surprising amount of nuance here. Hard to give a quick answer without a deeper look

I'll take a deeper look by the end of the weekend, if not sooner

On Thu, Sep 10, 2020, 7:46 AM Jason Scott notifications@github.com wrote:

Due to the breaking change beginning in Electron 9 https://www.electronjs.org/docs/breaking-changes#default-changed-loading-non-context-aware-native-modules-in-the-renderer-process-is-disabled-by-default in which allowRendererProcessReuse is set to true by default, specs that cover modules that use non-context aware native modules, like serialport https://www.npmjs.com/package/serialport, in the renderer will fail when using the karma-electron plugin:

Error: Loading non-context-aware native module in renderer: 'PATH_TO_PROJECT\node_modules\@serialport\bindings\build\Release\bindings.node', but app.allowRendererProcessReuse is true. See https://github.com/electron/electron/issues/18397.

at process.func [as dlopen] (electron/js2c/asar.js:140:31)

at Object.Module._extensions..node (internal/modules/cjs/loader.js:1034:18)

at Object.func [as .node] (electron/js2c/asar.js:140:31)

at Module.load (internal/modules/cjs/loader.js:815:32)

at Module._load (internal/modules/cjs/loader.js:727:14)

at Function.Module._load (electron/js2c/asar.js:769:28)

at Module.require (internal/modules/cjs/loader.js:852:19)

at require (internal/modules/cjs/helpers.js:74:18)

at bindings PATH_TO_PROJECT\node_modules\bindings\bindings.js:112:48)

at Object.<anonymous> (PATH_TO_PROJECT\node_modules\@serialport\bindings\lib\win32.js:1:129)

 Error: Cannot instantiate cyclic dependency! DatabaseService

at throwCyclicDependencyError (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:5438:1)

at R3Injector.hydrate (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:11344:1)

at R3Injector.get (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:11170:1)

at injectInjectorOnly (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:801:1)

at ɵɵinject (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:805:1)

at Object.StateService_Factory [as factory] (ng:///StateService/ɵfac.js:5:40)

at R3Injector.hydrate (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:11348:1)

at R3Injector.get (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:11170:1)

at NgModuleRef$1.get (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/core.js:23935:1)

at TestBedRender3.inject (http://localhost:9876/_karma_webpack_/node_modules/@angular/core/__ivy_ngcc__/fesm2015/testing.js:1930:1)

The transitional solution to this provided by the Electron team https://github.com/electron/electron/issues/18397 is to set allowRendererProcessReuse to false until version 11 of Electron but the option to set allowRendererProcessReuse to false does not appear to be exposed anywhere that can be set via a CLI argument or otherwise externally.

To temporarily fix this, I added

app.allowRendererProcessReuse = false;

to lib/electron-launcher.js and created a patch with patch-package https://www.npmjs.com/package/patch-package:

diff --git a/node_modules/karma-electron/lib/electron-launcher.js b/node_modules/karma-electron/lib/electron-launcher.js

index 28e779c..ac01a5a 100644

--- a/node_modules/karma-electron/lib/electron-launcher.js

+++ b/node_modules/karma-electron/lib/electron-launcher.js

@@ -35,6 +35,10 @@ app.on('window-all-closed', function handleWindowsClosed () {

app.quit();

});

+// Allow non-context aware modules to run in renderer in Electron 9+

+// https://github.com/electron/electron/issues/18397

+app.allowRendererProcessReuse = false;

+

// Update userData before Electron loads

// DEV: This prevents cookies/localStorage from contaminating across apps

app.setPath('userData', program.userDataDir);

This will keep our specs working until either any non-context aware dependencies catch-up to Electron's requirements or we move the usage of all of them to Electron's main process.

Is this something that would be appropriate to implement as an option in karma-electron? If so, I can write-up a PR.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/twolfson/karma-electron/issues/48, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG4KWCKB2NXM5IDAYRPDRLSFDRF5ANCNFSM4RFGGMKQ .

twolfson commented 4 years ago

Alright, looking into this more now

My rough understanding is this is only a temporary flag/item for us to support (e.g. Electron 9/10/11) but I'm not 100% certain of the timeline

twolfson commented 4 years ago

Alright, so notes out loud. Timeline via their corresponding PR: https://github.com/electron/electron/pull/18396

It seems like the pacing is about 3-4 months between releases, which gives us about 3-4 months until electron@11 is out and this patch is moot

The timeline feels too short to add support only to roll it back

Instead something like forking (npm supports git URLs with branches) or monkey-patching as you've done feels like the ideal way to go about this (or resolve the issues with the native modules)

If there's more demand around it though, then glad to reconsider adding support

Thanks for filing the issue! =)