nathanbuchar / electron-settings

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

Why delete watch method on new version #139

Closed fatoldsun00 closed 3 years ago

fatoldsun00 commented 3 years ago

Hi,

I analyse an electron app who use 3.2 of electron-settings. I see in code use of 2 methods : watch and emit. Correct me if i wrong, this methods are not part of the new version : class doesn't extends of eventEmitter and watch has disappeared.

Why ? perf killer ?

Thank you for your work !

Arjan-Zuidema commented 3 years ago

We still use an older version because of the removal of the watch feature.

nathanbuchar commented 3 years ago

Hi @fatoldsun00, check out the Reducing Complexity part of my blog post. I'd like to stress that I did not make the decision to remove .watch() lightly.

fatoldsun00 commented 3 years ago

Hi nathan, thank for your answer

My stance on settings observers now is that that sort of thing should really be handled by the developer. Electron Settings’ role is to act as a settings manager, not a settings nanny. While useful in many respects, I’m of the opinion nowadays that libraries shouldn’t do more than they need to.

It makes sense, that's what I was thinking

vanowm commented 2 years ago

With this logic - it's developers job to notify other parts of the code about changes in the settings - why bother with this module at all, since the developer might as well write their own settings module...Yet this library provides both sync and async methods, which I doubt both will be used in the same application. With your logic these should be 2 different libraries too.

Sorry, but I highly disagree with the reasoning for removing watchers. At least emit an event when a setting was been changed...