Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependency on 'remote' module #134

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

theArina
Copy link

@theArina theArina commented Oct 2, 2020

#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.

@theArina theArina changed the title Remove dependency on 'remote' module #133 Remove dependency on 'remote' module Oct 2, 2020
@nathanbuchar
Copy link
Owner

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
Copy link
Author

theArina commented Nov 17, 2020

@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
Copy link
Owner

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
Copy link
Author

@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
Copy link

MehediH commented Dec 17, 2020

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
Copy link

pierreraii commented Dec 28, 2020

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants