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

Electron 5 upgrade #175

Closed
whyboris opened this issue May 8, 2019 · 6 comments
Closed

Electron 5 upgrade #175

whyboris opened this issue May 8, 2019 · 6 comments
Labels
2.0.0 Priority for v2.0.0 release

Comments

@whyboris
Copy link
Owner

whyboris commented May 8, 2019

I've tried upgrading to Electron 5 but had some trouble. The Chromium window loads (with the loading animation) but the console shows some error (something like "Cannot load ____ of undefined" with stack trace showing something about zone and references no code I have written).

Things are further mildly complicated: Electron 5 uses Node 12 (which you can successfully update) but that requires to rebuild node-sass (which finally has binding for version 12) but is a bit of a pain (took about several attempts and about 5 minutes to get to build). We don't need to update to Node 12 to try out Electron 5, but I tried just in case and it didn't help with that error.

I tried updating all the packages (including zone.js and ts-node) but that didn't help with Electron 5 error. It feels like there is some API change in Electron, but it's not listed in the breaking changes log or releases.

The only change that broke the compilation was this (main.ts line 377):

shell.openExternal(urlToOpen, { activate: true }, (): void => {});

It expected 1 or 2 arguments, not 3, which was as simple fix:

shell.openExternal(urlToOpen, { activate: true });

I'm writing all this to document and to help with upgrading eventually. The base repository I based Video Hub App was angular-electron and it updated to Angular 5 with no problem:
maximegris/angular-electron#336 😅

So some import or method or something I'm doing is breaking Electron 5 🤷‍♂

@whyboris whyboris added 2.0.0 Priority for v2.0.0 release help wanted Extra attention is needed labels May 8, 2019
@whyboris
Copy link
Owner Author

whyboris commented May 8, 2019

Specifically, just updating to Electron 5 I get this error:

Uncaught ReferenceError: process is not defined
    at polyfills.js:5796
    at Function.push../node_modules/zone.js/dist/zone-mix.js.Zone.__load_patch (polyfills.js:2457)

Updating zone.js to version 0.9.1 results in:

Uncaught TypeError: Cannot read property 'eventNames' of undefined
    at eventTargetPatch (zone-mix.js:2460)
    at zone-mix.js:2828
    at Function.push../node_modules/zone.js/dist/zone-mix.js.Zone.__load_patch (zone-mix.js:99)

Makes me feel like some polyfills are missing, but the angular-electron project works fine, and has just one line:
https://github.com/maximegris/angular-electron/blob/master/src/polyfills.ts#L75
import 'zone.js/dist/zone';
Whereas we have
import 'zone.js/dist/zone-mix';
changing it to /zone creates a different error:

Uncaught ReferenceError: require is not defined
    at Object.url (external "url":1)
    at __webpack_require__ (bootstrap:78)

Probably better as was before 😓

Updated to Electron version 4.2.0 and created a PR to get ourselves as up to date as possible before the break happens with 5.0.0 so it's easier to trace out the problem 👌

@whyboris
Copy link
Owner Author

whyboris commented May 8, 2019

Looks like upgrading just zone.js from version 0.8.29 to 0.9.1 creates this error:

Uncaught TypeError: Cannot read property 'eventNames' of undefined
    at eventTargetPatch (zone-mix.js:2460)
    at zone-mix.js:2828
    at Function.push../node_modules/zone.js/dist/zone-mix.js.Zone.__load_patch (zone-mix.js:99)

So is may be an independent hurdle we'll need to overcome (not just Electron 5) 😅

@whyboris
Copy link
Owner Author

whyboris commented May 8, 2019

Updating just core-js from 2.6.4 to 3.0.1 results in this:

ERROR in ./node_modules/@angular-devkit/build-angular/src/angular-cli-files/models/jit-polyfills.js
Module not found: Error: Can't resolve 'core-js/es7/reflect' in '/Users/byakubchik/Desktop/VideoHub/Video-Hub-App/node_modules/@angular-devkit/build-angular/src/angular-cli-files/models'

I'd rather not mess with adding the reflect polyfill, seems like Angular 8 will handle this for us: angular/angular-cli#13954 (comment)

So we'll wait for Angular 8 to come out (sometime within this/next week 🎉 )

@whyboris
Copy link
Owner Author

whyboris commented May 8, 2019

Now that I've updated as much as I could with #176 -- I tried Electron 5.0.1 again and still same as above. Attaching everything for completeness, two console errors in Chrome Dev Tools on start:

polyfills.js:5796 
Uncaught ReferenceError: process is not defined
    at polyfills.js:5796
    at Function.push../node_modules/zone.js/dist/zone-mix.js.Zone.__load_patch (polyfills.js:2457)
    at polyfills.js:5794
    at polyfills.js:2368
    at Object../node_modules/zone.js/dist/zone-mix.js (polyfills.js:2370)
    at __webpack_require__ (runtime.js:79)
    at Module../src/polyfills.ts (polyfills.js:5892)
    at __webpack_require__ (runtime.js:79)
    at Object.1 (polyfills.js:5938)
    at __webpack_require__ (runtime.js:79)

external "url":1 
Uncaught ReferenceError: require is not defined
    at Object.url (external "url":1)
    at __webpack_require__ (bootstrap:78)
    at Object.<anonymous> (client:6)
    at Object../node_modules/webpack-dev-server/client/index.js?http://0.0.0.0:0/sockjs-node (vendor.js:105358)
    at __webpack_require__ (bootstrap:78)
    at Object.0 (polyfills.ts:40)
    at __webpack_require__ (bootstrap:78)
    at checkDeferredModules (bootstrap:45)
    at Array.webpackJsonpCallback [as push] (bootstrap:32)
    at main.js:1

I'll do some digging, but if anyone has come across this, please share your thoughts 👍

@whyboris whyboris removed the help wanted Extra attention is needed label May 9, 2019
@whyboris
Copy link
Owner Author

whyboris commented May 9, 2019

Problem solved:

https://stackoverflow.com/questions/55093700/electron-5-0-0-uncaught-referenceerror-require-is-not-defined

It turns out, nodeIntegration was true by default in previous electron versions, but false by default in 5.0.0.

Just needed this:

new BrowserWindow({
    webPreferences: {
      nodeIntegration: true,

@whyboris
Copy link
Owner Author

whyboris commented May 9, 2019

Closed with #176

@whyboris whyboris closed this as completed May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.0.0 Priority for v2.0.0 release
Projects
None yet
Development

No branches or pull requests

1 participant