Update to Electron 7 #1157

stackptr · 2019-10-31T02:31:58Z

I was complaining about a system vibrancy problem in #1091 and it was indicated that to fix the issue properly on Catalina it was necessary to wait for the next version of Electron to be released. It was indeed released a week ago.

This PR updates the version and then fixes the errors that I saw running npm install && npm start.

(It does not appear to fix my original complaint, any additional work will go in another branch)

It doesn't seem like `RequestHeaders` is provided by `electron` anymore. From docs on `webRequest.onBeforeSendHeaders` [1], `requestHeaders` is typed as an optional `Record<string, string>`. 1: https://electronjs.org/docs/api/web-request#webrequestonbeforesendheadersfilter-listener

This interface is no longer present in `electron` but it spelled out in the docs [1] 1: https://electronjs.org/docs/api/web-request#webrequestonsendheadersfilter-listener

This might've been needed if `requestHeaders` wasn't defined on the prior `OnSendHeadersDetails` interface.

`shell.openExternalSync` was deprecated in electron/electron#18179 Since it is no longer present, it should be replaced with `shell.openExternal`. This is not a synchronous function and since the full context of these functions is unclear to me, `async`/`await` syntax is used.

stackptr · 2019-10-31T02:34:40Z

There are some non-blocking warnings that I will also try to address:

> tsc && electron .

(electron) 'getName function' is deprecated and will be removed. Please use 'name property' instead.
(electron) 'systemPreferences.isDarkMode()' is deprecated and will be removed. Please use 'nativeTheme.shouldUseDarkColors' instead.
(electron) 'setAppLevelAppearance function' is deprecated and will be removed. Please use 'appLevelAppearance property' instead.
(electron) 'setBadgeCount function' is deprecated and will be removed. Please use 'badgeCount property' instead.

AidanGG · 2019-10-31T12:14:05Z

This fixes my issue that I opened up yesterday too #1153 😄.

stackptr · 2019-10-31T17:21:37Z

It's unclear to me how to resolve the lint error on @typescript-eslint/no-unused-vars for the IpcMain class, but since it's a hack that seems to be resolved in electron/electron#20712 maybe the thing to do is to wait for the minor version that fixes it? I'm not quite sure in what version that would be expected to land.

sindresorhus · 2019-10-31T18:17:08Z

Just use a // @ts-ignore comment to work around it.

This supresses the following error when using `npm run lint`: ``` ✖ 20:8 IpcMain is defined but never used. @typescript-eslint/no-unused-vars ``` Note that an `// @ts-ignore` directive did not supress this error, I guess because the linting engine `xo` performs typescript linting with `eslint`.

stackptr · 2019-10-31T21:03:57Z

Dark mode following system appearance doesn't have to go in this PR, but pretty much all the existing logic for it seems to have been deprecated. I've been trying to get this to use the new nativeTheme api and I've finally determined that it is not importable in source/browser.ts where setDarkMode() lives.

The import statement import {nativeTheme} from 'electron' does not yield type errors it just turns out undefined when trying to use it. After some digging, I see that the different "processes" have a module listing in the main electron repo, lib/${PROCESS}/api/module_list.ts. So, for browser.ts the process is renderer, which lacks any mention of the relevant nativeTheme api. It is however accessible in source/index.ts, which could be the browser process. (Which seems to be referred to as Main in the electron docs).

Anyways, it appears some code needs to be shuffled around here. I can send nativeTheme in the set-dark-mode event in the webContents EventEmitter but then there is the setDarkMode call in the DOMContentLoaded event that would also need it. I'll poke around further but any ideas regarding this architecture would be helpful!

sindresorhus · 2019-11-01T03:42:14Z

You could use https://github.com/sindresorhus/electron-util#api-1

sindresorhus · 2019-11-01T03:43:19Z

source/index.ts

+	resourceType: string;
+	referrer: string;
+	timestamp: number;
+	requestHeaders: Record<string, string>;
 }


Why do we have to manually type this now?

It doesn't seem to be present in the electron library. I'm not sure why

source/emoji.ts

source/index.ts

package.json

source/emoji.ts

This was fixed in electron 7.1.1

From `npm run lint`: ``` source/index.ts:466:5 - error TS2532: Object is possibly 'undefined'. 466 options.webPreferences.nodeIntegration = false; ~~~~~~~~~~~~~~~~~~~~~~ source/index.ts:467:5 - error TS2532: Object is possibly 'undefined'. 467 options.webPreferences.preload = path.join(__dirname, 'browser-call.js'); ~~~~~~~~~~~~~~~~~~~~~~ ```

stackptr · 2019-11-11T02:05:19Z

Addressed merge conflicts, resolved some questions and using later Electron version to avoid that type hack.

sindresorhus · 2019-11-11T09:41:10Z

Thanks 🙌

stackptr added 7 commits October 30, 2019 18:41

Update electron dep to 7.0.0

d89e3b7

Add override to fix incorrectly typed removeListener method

a0c5b5a

Define OnSendHeadersDetails interface

9208989

This interface is no longer present in `electron` but it spelled out in the docs [1] 1: https://electronjs.org/docs/api/web-request#webrequestonsendheadersfilter-listener

Remove unnecessary any

ea15933

This might've been needed if `requestHeaders` wasn't defined on the prior `OnSendHeadersDetails` interface.

Support optional webPreferences on options object

0bcb724

Fix formatting errors

8b35f41

sindresorhus requested changes Nov 1, 2019

View reviewed changes

sindresorhus reviewed Nov 8, 2019

View reviewed changes

package.json Outdated Show resolved Hide resolved

sindresorhus reviewed Nov 8, 2019

View reviewed changes

source/emoji.ts Outdated Show resolved Hide resolved

sindresorhus changed the title ~~Update Caprine to Electron 7.0.0~~ Update to Electron 7 Nov 8, 2019

sindresorhus mentioned this pull request Nov 8, 2019

Can't open links #1166

Closed

stackptr added 3 commits November 10, 2019 17:53

Merge branch 'master' into electron-v7

674979d

Remove custom module declaration

abfb27c

This was fixed in electron 7.1.1

sindresorhus merged commit 833c42b into sindresorhus:master Nov 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to Electron 7 #1157

Update to Electron 7 #1157

stackptr commented Oct 31, 2019 •

edited

Loading

stackptr commented Oct 31, 2019

AidanGG commented Oct 31, 2019

stackptr commented Oct 31, 2019

sindresorhus commented Oct 31, 2019

stackptr commented Oct 31, 2019

sindresorhus commented Nov 1, 2019

sindresorhus Nov 1, 2019

stackptr Nov 1, 2019

stackptr commented Nov 11, 2019

sindresorhus commented Nov 11, 2019

Update to Electron 7 #1157

Update to Electron 7 #1157

Conversation

stackptr commented Oct 31, 2019 • edited Loading

stackptr commented Oct 31, 2019

AidanGG commented Oct 31, 2019

stackptr commented Oct 31, 2019

sindresorhus commented Oct 31, 2019

stackptr commented Oct 31, 2019

sindresorhus commented Nov 1, 2019

sindresorhus Nov 1, 2019

Choose a reason for hiding this comment

stackptr Nov 1, 2019

Choose a reason for hiding this comment

stackptr commented Nov 11, 2019

sindresorhus commented Nov 11, 2019

stackptr commented Oct 31, 2019 •

edited

Loading