Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
EngJay opened this issue Sep 10, 2020 · 3 comments
Closed

Comments

@EngJay
Copy link

EngJay commented Sep 10, 2020

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 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:

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
Copy link
Owner

twolfson commented Sep 10, 2020 via email

@twolfson
Copy link
Owner

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
Copy link
Owner

Alright, so notes out loud. Timeline via their corresponding PR: electron/electron#18396

  • 5 major versions, about 60 weeks from May 21 2019

    • 52 weeks for May 21 2020, 8 more weeks puts us in July 2020 so we're already 2 months overdue
  • Electron 9: Changes default value, requires overriding for old behavior

  • Electron 10: Adds deprecation warning

  • Electron 11: Removes ability to override

  • Electron version lifespans

  • All Electron versions via npm show electron versions

  • electron@9.0.0-beta.1, released 7 months ago (so Feb 2020)

  • electron@9.0.0, released 3 months ago (so Jun 2020)

  • electron@10.0.0-beta.1, released 3 months ago (so Jun 2020 also)

  • electron@10.0.0, released 2 weeks ago (so Aug 2020)

  • electron@11.0.0-beta.1, released 3 months ago (so Aug 2020 also)

  • For curiosity, here's 7.0 and 8.0 as well

    • electron@7.0.0, released 10 months ago (so Nov 2019)
    • electron@8.0.0, released 7 months ago (so Feb 2020)

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! =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants