nathanbuchar / electron-settings

📝 A simple persistent user settings framework for Electron.
https://electron-settings.js.org
MIT License
814 stars 60 forks source link

Remove dependency on 'remote' module #134

Open theArina opened 3 years ago

theArina commented 3 years ago

133

Here is my fork, i was making it hastily and expect it to be edited. The main idea is to replace 'remote' module with ipc messaging as it is advised in Electron 7. To make it work now you should call settings.init() in the main process.

nathanbuchar commented 3 years ago

Hi @theArina, thanks for scaffolding this all out. This is quite a bit more involved than I was envisioning, and it adds a whole new step to using the lib (.init()).

Attempting to eliminate friction, is there any reason that init() needs to be called by the developer? Couldn't those ipc listeners be bound during some sort of automatic bootstrapping process rather than an explicit invocation?

Also, I'm curious as to why your pull request removes dist from the .gitignore and removes .npmignore entirely. Is there a specific reason for this?

theArina commented 3 years ago

@nathanbuchar, yes, there was a reason for removing dist and .npmignore, but just to be able to use that fork, you could just put it back like it was. Generally, you could just take an idea of this pull request and make it as you consider better. As for 'init' method, I personally couldn't think of anything else here to make, cause 'remote' module won't be a good thing anymore, and this is what allowed to connect to the main process not explicitly.
Anyway, maybe you'll come up with a better idea :) P.S. I also think that it's better to work with files only from main process, this is why I moved all of it from the renderer.

nathanbuchar commented 3 years ago

Currently building a variation of this into the lib right now, but one thing I noticed: doesn't this require that electron-settings is initialized in the main process before it can be used in the render process? As it stand right now, electron-settings can be used whenever and wherever, without having a dependency of first initializing it in the main process. Adding this in would be a breaking change, and would require bumping to v5.

All electron settings needs to use remote for is to access the app and pull out the userData path. I like the idea of loading/saving settings by proxy, and initiating them via IPC in render processes, but I don't know if I can justify such a breaking change. I haven't been following Electron in a little while, which is why I'm slow to update this, so forgive me if I'm a little out of touch but is there really no other way around this?

At the very least, has calling init() on a module in the main process at least become a common, predictable pattern for libraries such as this?

theArina commented 3 years ago

@nathanbuchar, okay, i'm glad to help you somehow, but i'm really just a user of your library who needed to update Electron to the last version. So, unfortunately i don't know if init() is a common pattern, i just didn't see any other way to do that. As for Electron updates, you can refer to here. Also, i've noticed that @electron/remote uses initialize() method which is kinda answering your question if it's a good idea to use init(). You could make a version that throws deprication about this first and then make another one with this breaking change, there is some time till 14 Electron release where remote will be removed.

MehediH commented 3 years ago

hey @nathanbuchar, just wanted to check in here to see if you have any updates on this PR or your own implementation? I have been thinking of working around this issue but can not think of a way to do so without the init() idea.

pierreraii commented 3 years ago

@nathanbuchar @theArina If the only dependency on remote module is to get the user data path, electron-json-storage made a pretty good solution for this, which is any developer wanting to use the library without the remote module, should get the user data path through IPC or any other way from the main into the renderer process, and inject it into the library after requiring it and before using it there.

All that would be needed is a new method to be able to provide a custom user data path, and for sure avoid any error caused by the non-existence of the remote module.

Reference: https://github.com/electron-userland/electron-json-storage#running-on-electron-10-renderer-processes