Remove remote

[dusansimic]

Closes: #36
Closes: #45
Closes: #49
Closes: #52

[lacymorrow]

Yes! Thank you @dusansimic, this brings new life into this project now that future electron versions are supported.

@sindresorhus I understand you don't work much with Electron anymore, would you be willing to look this over or grant another person maintainer access?

[dusansimic]

Hi @lacymorrow, keep in mind that this pr still has some work left. I'd like to update libraries dependencies (including linter and other test libraries). I just wanted to publish this as a draft pr in case if anyone else is interested in taking a look, especially since this is going to be quite an update for the library so the more eyes we have on this the better.

At the moment I'm trying to get CI to work so it might take some time.

[sindresorhus]

@dusansimic There has not been any activity on this for a long time. Time to close it? Or do you still intend to finish it some day?

[dusansimic]

I do plan on finishing it. I just reached a few blockers regarding maintaining the api compatibility I wasn't sure how to solve and it could buried under other things. Most likely course of action I can see is rewriting the API in TS since it's anyway going to break things and people will need to rewrite things in their apps.

It will then be compiled to js + ts definitions before being pushed to npm.

The logic is essentially there, it just needs to be packaged into a sane api. I'll try to do it in the first part of July if that sounds okay?

[sindresorhus]

I just reached a few blockers regarding maintaining the api compatibility I wasn't sure how to solve and it could buried under other things

We can make breaking changes where it makes sense.

Most likely course of action I can see is rewriting the API in TS.

Sure

I'll try to do it in the first part of July if that sounds okay?

👍

[dusansimic]

So far the progress is the following:

• Port library to TypeScript and split functions into main and shared module (main runs only in main process and shared can run in any process).
• Bump some dependencies (ava, electron, typescript, tsd, xo)
• Add some new dependencies (prettier + prettier xo config)
• Port type tests (tsd)
• Port example app
• Port unit tests (util.platform)
• Verify npm package will publish correctly

Regarding the unit test (platform function), I wasn't able to port it in ava since I couldn't modify the chrome version property in the process.versions object. I tried to implement the test in playwright but for some reason I had no success with it. I will most likely implement it in the example app so it will require manual testing but it's going to work at least.

[sindresorhus]

I wasn't able to port it in ava since I couldn't modify the chrome version property in the process.versions object

Try with Object.defineProperty.

[dusansimic]

Try with Object.defineProperty.

Thanks, I've tried that and it didn't work for some reason. I tried putting it into the testing context (the test function callback) but no luck. I'll still look for solutions for that but for now I've implemented that test in the example app. It should work out of the box when the app is started using npm run start.

Other than that, I've changed up various types and worked on cleaning up some code. I'll also update the docs for the functions including the example snippets. I'm not completely sure about some changes so I'll write about them tomorrow.

[dusansimic]

I added error throws whenever there is a possibility that there is no active window (which is important for functions like getWindowBoundsCentered). This seemed like the most sane solution to me.

[sindresorhus]

The readme must be updated.

[sindresorhus]

We also need to document which API must be called in the main and renderer process.

[dusansimic]

We also need to document which API must be called in the main and renderer process.

Do you recommend any docs generators (TS defniitions to docs like those already in readme)?

[sindresorhus]

Do you recommend any docs generators (TS defniitions to docs like those already in readme)?

Not really. TSDoc is not very good. I generally prefer to just duplicate between readme and doc comments.

[sindresorhus]

@dusansimic Friendly bump. It would be great to get this merged. We can leave non-essential things for later.

[dusansimic]

Sorry for the delay again. Would you consider using a tool like tsdocs.dev for api docs instead of manually maintaining the docs in the repo?
Here is how the page for electron-util currently looks on the website link.

[sindresorhus]

Would you consider using a tool like tsdocs.dev for api docs instead of manually maintaining the docs in the repo?

Yeah, that makes sense.

[lacymorrow]

Is there no way to have this work with the renderer route without remote? It would allow supporting Electron <12.

[dusansimic]

@lacymorrow

The reason for this pr is to introduce support for this module in versions >=12 because at the moment, it's not compatible with them at all. The current latest version of electron-util is supported in Electron versions <12.

I've decided to not use remote module in order to not force it on the users of this module. If users want to access api that is in main process only, they can create ipc channels to get results from those function calls which is what Electron recommends at the moment. It is a relatively big breaking change but it's in line with what Electron recommends.

This has already been summed up quite nicely in the previous pr which didn't go through #37 (comment).

It is possible for us to create those ipc channels and emulate behavior of remote but in my opinion, the less magic this module does the better.

[lacymorrow]

To be clear, I'm in favor of deprecating the remote module and limiting the features available in the renderer process in Electron >= 12, however there doesn't seem to be any reason why this new version cannot be backwards-compatible with older versions. Will it still work the same for Electron < 12?

[sindresorhus]

Will it still work the same for Electron < 12?

No, the plan is to target a newer Electron version, probably Electron 27.

[lacymorrow]

I believe by doing something like:

const Electron = require('electron')
if (Electron.remote) {
  // proceed as normal, support Electron < 12
}  

or by using process.versions.electron it should be possible to have this change remain backwards-compatible.

Some of us are locked into a specific Electron version due to native modules or other reasons.

[sindresorhus]

It's probably possible to provide backwards compatibility with such an old Electron version, but that's not something I'm interested in providing. It's more than enough work tracking just the lastest-1 of Electron. Electron changes and breaks so much all the time.

[sindresorhus]

Looking good to me. Thanks for working on this, @dusansimic 🙏