Kap icon indicating copy to clipboard operation
Kap copied to clipboard

Replace remote usage with @electron/remote

Open timothyis opened this issue 3 years ago • 6 comments

This pull request replaces all usage of the remote module – built in to Electron before v14 – with the new @electron/remote module.

I have tested this, but I'm not sure if I've missed any edge-cases.

Note: We should work on replacing all usage of this module with other methods, such as using ipc instead.

Closes #1089.

timothyis avatar Feb 18 '22 02:02 timothyis

From the migration guide:

Note: In electron >= 14.0.0, you must use the new enable API to enable the remote module for each desired WebContents separately: require("@electron/remote/main").enable(webContents).

  • https://github.com/electron/remote#readme

sindresorhus avatar Feb 18 '22 08:02 sindresorhus

@sindresorhus I haven't upgraded Electron first, I was going to do that in a separate PR in case more changes were required for the upgrade and this PR got too big. I think in this case, that line of code requires Electron 14+ because it breaks the app when used.

timothyis avatar Feb 18 '22 14:02 timothyis

Scratch that, the issue is that electron-util also uses remote, and removing the config for webPreferences breaks the module: https://github.com/sindresorhus/electron-util/issues/36

timothyis avatar Feb 18 '22 14:02 timothyis

How big of a change would it be to move away from electron-util?

cgarrovillo avatar Mar 09 '22 20:03 cgarrovillo

That's what I'm working on instead now @cgarrovillo. I've updated Electron locally so I've been removing it piece by piece. However, Electron's DX has changed substantially since I last touched this codebase so it's an interesting task. I'll push what I have soon since it renders the app. There's just a few broken things left.

timothyis avatar Mar 10 '22 10:03 timothyis

Awesome. I'll be taking a look at the Kap codebase this weekend too so I'll see if I can help out in any way.

cgarrovillo avatar Mar 10 '22 16:03 cgarrovillo

Closing this to hopefully take a fresh look. I overextended in this PR and ended up trying to do too many things (especially in my local code.)

timothyis avatar Oct 26 '22 15:10 timothyis