Open vik-13 opened 3 years ago
We don't have to explicitly run change detection if we run the IPC bus inside of the Angular zone.
Probably I did not do this to not trigger redundant cycles.
I'd also suggest keeping the promise API for simplicity to keep the project's contribution curve low.
Once we use OnPush strategy for all components extra cycles won't be a performance issue anymore... Especially as in most cases when we receive any data from electron we need to show it so we need to run change detection. In another hand as you mentioned "keep the project's contribution curve low" it's not obvious that we need to run cd manually sometimes, it could cause extra cycles as well. It's something like a bit lower performance (really a bit) against usability.
About keeping promise API. Currently we have promise API for .send() and callback for .on(). I understand what do you mean, but as angular uses Observables by default (HttpClient, Routes, EventEmitter...) you still need to understand how it works and use that..even if not in the best way. As for me it would be great to make it more consistent, maybe even something like Redux flow... dispatch an action and subscribe to streams. F.ex. Currently it's possible to call .send() get the result and in another place call .on() and get the result again... It's petty confusing. And using RxJS makes it more popular and let users use it more :)
I'm thinking about refactoring IPCBus to use observables instead of promises;
ipc.ts is more simple and easier to use, but has one disadvantage...It runs outside the angular zone and detectChanges() should be called all the time.
ipc.service.ts solution could solve it. So no need to care about running change detection flow. It would be enough markForCheck() or use async pipe. But this way requires state-manager.ts refactoring as we have this.state = new StateProxy().
I'm gonna refactor it if it's fine for you.