electron-json-storage icon indicating copy to clipboard operation
electron-json-storage copied to clipboard

compatibility with electron 10

Open Tobias-Keller opened this issue 5 years ago • 7 comments

tested today the packe with electron 10 and only can use it if i activate the "enableRemoteModule" webPreference.

in electron 10 they changed the default value of "enableRemoteModule" to false, because there are better ways.

will this be updated in this package, so electron-json-storage uses better methods?

Tobias-Keller avatar Sep 05 '20 15:09 Tobias-Keller

Hi @Tobias-Keller ,

Thanks for the heads up. I haven't been involved in the Electron.js community lately, but I'll do some research. In the mean-time, what are those better methods that you are mentioning?

Do you have a rough idea of how things should work on Electron.js 10, and how to add this in a backwards compatible way to this module?

jviotti avatar Sep 09 '20 15:09 jviotti

instead of using the remote module, we can use the ipc ipc compatibility in the main process: https://www.electronjs.org/docs/api/ipc-main/history ipc renderer compatibility: https://www.electronjs.org/docs/api/ipc-renderer/history

why? read this: https://medium.com/@nornagon/electrons-remote-module-considered-harmful-70d69500f31

Tobias-Keller avatar Sep 09 '20 16:09 Tobias-Keller

So the only place that we rely on this is in order to get the userData path:

const electron = require('electron');
const app = electron.app || electron.remote.app;
...
exports.getDefaultDataPath = function() {
  return path.join(app.getPath('userData'), 'storage');
};

Doing this with IPC would overly complicate this module, as you would need to setup the module in the main process before using it the renderer process instead of just require()ing as it is now. Let me see if I can come up with a another solution...

jviotti avatar Oct 05 '20 14:10 jviotti

OK, so I don't think its going to be easy to completely get rid of remote while letting the module be easily setup. At least for the time being, I'll update the docs to clarify that if you want to run this module on a renderer process without using remote, then you can manually call .setDataPath() on the renderer process (assuming you obtained the data path through a custom method) before calling any other functions. Doing so will make the module not go through remote.

jviotti avatar Oct 05 '20 14:10 jviotti

Actually, are you sure this deprecation has been decided? It seems like it is still under discussion based on https://github.com/electron/electron/issues/21408, and it seems to work fine for me.

What exact Electron version are you testing this on? Is it actually failing for you? Can you provide a test app?

jviotti avatar Oct 05 '20 14:10 jviotti

Actually, are you sure this deprecation has been decided? It seems like it is still under discussion based on electron/electron#21408, and it seems to work fine for me.

What exact Electron version are you testing this on? Is it actually failing for you? Can you provide a test app?

its in the breaking changes on version 10: https://www.electronjs.org/docs/breaking-changes#default-changed-enableremotemodule-defaults-to-false

so i think you can reproduce this with every electron 10 app without setting enableremotemodule to true manually

Tobias-Keller avatar Oct 05 '20 15:10 Tobias-Keller

Right, my bad. I was still accidentally running on v9. I'll update the README

jviotti avatar Oct 05 '20 15:10 jviotti