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

[questions] Is Electron 12 supported? #50

Closed
prince-chrismc opened this issue Mar 26, 2021 · 16 comments
Closed

[questions] Is Electron 12 supported? #50

prince-chrismc opened this issue Mar 26, 2021 · 16 comments

Comments

@prince-chrismc
Copy link

"electron": "~11.2.2",

I was trying to upgrade locally but I ran into this issue.

26 03 2021 16:02:24.565:INFO [karma-server]: Karma v6.3.1 server started at http://localhost:9876/
26 03 2021 16:02:24.566:INFO [launcher]: Launching browsers AngularElectron with concurrency unlimited
26 03 2021 16:02:24.574:INFO [launcher]: Starting browser Electron
√ Browser application bundle generation complete.
26 03 2021 16:02:44.268:INFO [Electron 12.0.2 (Node 14.16.0)]: Connected on socket rmlyL4_NcCgYp2qTAAAB with id 14501640
Electron 12.0.2 (Node 14.16.0) ERROR
  An error was thrown in afterAll
  Uncaught ReferenceError: require is not defined
  ReferenceError: require is not defined

My release build and local builds works, it's only my unit tests that fail.

I started from a boilerplate but as I understand it, this plugin provides Karma with the ability to test with require so hopefully this is a good place to ask.

If you have any suggestions that would be appreciated also!

@twolfson
Copy link
Owner

It definitely should be supported. Unsure what's causing that error. Will triage by the end of the weekend

@prince-chrismc
Copy link
Author

Thanks, I'd really appreciate it!

@twolfson
Copy link
Owner

Looking into this now

@twolfson
Copy link
Owner

I've reproduced electron@12 failing, though the error message I'm seeing from npm test isn't quite what you've got

26 03 2021 19:57:58.898:INFO [launcher]: Launching browser ElectronWithNodeIntegration with unlimited concurrency
26 03 2021 19:57:58.902:INFO [launcher]: Starting browser Electron
26 03 2021 19:57:59.300:INFO [Electron 12.0.2 (Node 14.16.0)]: Connected on socket FInyO26cds6C8OkIAAAA with id 69134456
26 03 2021 19:58:01.302:WARN [Electron 12.0.2 (Node 14.16.0)]: Disconnected (1 times), because no message in 2000 ms.
Electron 12.0.2 (Node 14.16.0) ERROR
  Disconnected, because no message in 2000 ms.


npm ERR! code ELIFECYCLE
npm ERR! errno 1

@twolfson
Copy link
Owner

When I run in continuous mode (npm run test-karma-continuous), it does show the same error -- though oddly only in the browser console

@twolfson
Copy link
Owner

Adding contextIsolation: false to my webPreferences fixes it. Now I need to read up more on that and:

  • Figure out if that's the right fix
  • Figure out if it's something to tell people to use, build into the repo, or both

https://stackoverflow.com/a/55908510/1960509

@twolfson
Copy link
Owner

It looks like contextIsolation started defaulting to true in Electron@12 so it's most likely the correct switch:

https://www.electronjs.org/docs/tutorial/context-isolation

It does seem related to nodeIntegration which we've documented in our README as an explicit opt-in

https://github.com/twolfson/karma-electron/tree/6.3.3#forcing-nodeintegration-support

That was regarding a workaround for preload though and preload was introduced in Electron@5 back 2 years ago:

https://www.electronjs.org/blog/electron-5-0

Digging into my own README now...

@twolfson
Copy link
Owner

twolfson commented Mar 27, 2021

Since we don't set loadScriptsViaRequire to true by default, it seems like people aren't meant to have a guarantee for window.require, so it does sound like this is another option to document in the examples

But still pushing people towards using preload instead of these overrides

Going to still keep on reading a bit more (maybe play with preload in tests) but prob settling on that

@twolfson
Copy link
Owner

Yea, I think the thought process is:

  • People will want to stay 1:1 with their existing Electron setup, so if they're using their webPreferences with nodeIntegration and contextIsolation already set, then they'll keep using it
  • Otherwise, they'll figure out their own solution
  • But since we're a test runner, we're not prescriptive

Going ahead with documentation only

@twolfson
Copy link
Owner

This has been documented in 6.3.4. Thanks for raising the issue =)

https://github.com/twolfson/karma-electron/tree/6.3.4#forcing-nodeintegration-and-contextisolation-support

@twolfson
Copy link
Owner

Hmm, I might have spoken too soon -- test-karma-no-node-integration is failing in CI

@twolfson twolfson reopened this Mar 27, 2021
@twolfson
Copy link
Owner

Hmm, yea -- I think missing contextIsolation is also affecting the karma-electron bridge to the terminal somehow (which is why my console.log never were showing up

I'm out of time to dig into this tonight but we do have a workaround for now as documented:

// Inside custom launcher
browserWindowOptions: {
  webPreferences: {
    contextIsolation: false
  }
}

@prince-chrismc
Copy link
Author

Ahhh thank you so much, reading the example I discovered this was related to the Karma.conf.js not my main.ts 🤯

I had one other unrelated problem and I have my unit tests successfully running know

🤗 Thank you for the help!

@twolfson
Copy link
Owner

Glad to hear that got it working =D Going to leave this issue open as we've still got broken CI to look into

@twolfson
Copy link
Owner

Started digging into this again, wanted to find out exactly what part of contextIsolation was breaking (e.g. we're not really sending any traffic between the main electron process and our renderer)

After some digging, saw some postMessage events which weren't receiving origin (which is good for security checks)

It turns out this is part of contextIsolation but apparently there's another flag we can set:

{
  webPreferences: {
    nativeWindowOpen: true
  }
}

which also seems to resolve the issues for a require-free Electron:

electron/electron#9340 (comment)

https://www.electronjs.org/docs/api/window-open#using-chromes-windowopen-implementation

Going to more robustly test that setting (maybe even rollback the contextIsolation wording) and update the documentation (if it all seems accurate)

@twolfson
Copy link
Owner

Alright, settled on nativeWindowOpen: true as default always (released in 7.0.0, just in case it's a breaking change)

and updated documentation to better explain 2 getting started mechanisms (non-extended config, Node.js + custom config)

@twolfson twolfson added the bug label Mar 27, 2021
Hercules2013 added a commit to Hercules2013/angular-electron that referenced this issue Oct 24, 2023
Boss930129 added a commit to Boss930129/angular_electron that referenced this issue Jun 6, 2024
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