sindresorhus / electron-unhandled

Catch unhandled errors and promise rejections in your Electron app
MIT License
448 stars 26 forks source link

Plans to remove use of the remote module? #22

Closed Nantris closed 2 years ago

Nantris commented 3 years ago

contextIsolation is about to become the default setting with Electron@12. Remote is already disabled by default and will be removed in, I believe, Electron@14.

Are there plans to support these changes?

Thanks for sharing your great work @sindresorhus

sindresorhus commented 3 years ago

Is this package incompatible with contextIsolation?

As for the remote thing. I think we'll have to do something like https://github.com/sindresorhus/electron-store/commit/191ae04338f375a75899083217ea6ecd05dc7f51 or depend on https://github.com/electron/remote, but the former is preferred.

It's not something I plan to work on though. I don't use Electron anymore. But I'm happy to review a good pull request.

Relevant discussion: https://github.com/sindresorhus/electron-util/issues/36#issuecomment-749622810


// @dertieran Just FYI.

Nantris commented 3 years ago

Thanks for your reply @sindresorhus I think you're right that the issue is only the use of remote, no problem with contextIsolation.

Now I have to ask though, have you moved on from Electron to something else? Thanks for your enormous contributions to the Electron community!

sindresorhus commented 3 years ago

Now I have to ask though, have you moved on from Electron to something else? Thanks for your enormous contributions to the Electron community!

Native app development with Swift.

I still intend to maintain my Electron packages, but things like this issue are not my top priorities.

Nantris commented 3 years ago

Thanks for the info, I'll have to keep more of an eye on Swift.

Would you be open to a PR that updated to not need remote if it also removed electron-is-dev? I noticed that package also relies on remote. We could update the README to point people to that package which they could implement themselves if they'd like - this way the two packages could be maintained more independently. Thoughts?

We don't use electron-is-dev and an elegant method to remove remote from that package doesn't jump out to me.

sindresorhus commented 3 years ago

electron-is-dev should be changed to only work in the main process.

If we do like in https://github.com/sindresorhus/electron-store/commit/191ae04338f375a75899083217ea6ecd05dc7f51, it should be easy to get the electron-is-dev data from the main process too.

Nantris commented 3 years ago

@sindresorhus I don't see a decent way to handle the dialog and clipboard features without substantially changing the api. I'd forgotten about these as we don't use them. Any recommendations?

dertieran commented 3 years ago

I think a solution could be to just send all errors from the renderer to the main process and handle them all there.

Nantris commented 3 years ago

Am I wrong to think that an error could potentially contain non-serializable data which would make that impossible?

I'm also not experienced with TypeScript so the more substantial the changes the less likely I'm going to be able to make these changes in a way that will be easily merged back into the repo.


Unrelated to the above, @sindresorhus what's the purpose of the debounce here? Might be useful to add a comment as to why it's needed.

https://github.com/sindresorhus/electron-unhandled/blob/main/index.js#L82-L84

sindresorhus commented 3 years ago

I think a solution could be to just send all errors from the renderer to the main process and handle them all there.

Yup, that seems like the best solution.

Am I wrong to think that an error could potentially contain non-serializable data which would make that impossible?

It potentially could, but we should just strip out such data as it's not relevant to reporting errors.

sindresorhus commented 3 years ago

Unrelated to the above, @sindresorhus what's the purpose of the debounce here? Might be useful to add a comment as to why it's needed.

I should probably have added a code comment there, but git blame is your friend: https://github.com/sindresorhus/electron-unhandled/commit/78a5af8cdfd770c1206ec49d80af564a15a7b501

Nantris commented 3 years ago

Great tip on git blame. Being a solo dev, I need to get into a better habit with that.

Do you have any recommendation for stripping out non-serializable data?