-
Notifications
You must be signed in to change notification settings - Fork 12
feat(typed-ipc): main process IpcListener cleanup functions
#20
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
base: master
Are you sure you want to change the base?
Conversation
The main process `IpcListener` now returns cleanup functions for its `on` and `handle` methods which remove the registered listener or handler. Its `dispose` method now unsubscribes only listeners and handlers registered with this class instance. Closes alex8088#19
| /** | ||
| * Dispose all listeners and handlers. | ||
| * Dispose all registered listeners and handlers. | ||
| */ | ||
| dispose(): void { | ||
| this.listeners.forEach(c => ipcMain.removeAllListeners(c)) | ||
| this.listeners = [] | ||
| this.handlers.forEach(c => ipcMain.removeHandler(c)) | ||
| this.handlers = [] | ||
| this.listeners.forEach((listeners, channel) => { | ||
| listeners.forEach(listener => ipcMain.removeListener(channel, listener)) | ||
| }) | ||
| this.listeners.clear() | ||
| this.handlers.forEach(channel => ipcMain.removeHandler(channel)) | ||
| this.handlers.clear() | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Old behavior:
disposeremoves all listeners for each channel, even those which were not registered by the instance. - New behavior:
disposeremoves listeners registered by this specific instance.
I think the new behavior is less surprising than the old behavior, but you could argue that this is a breaking change.
| private listeners: string[] = [] | ||
| private handlers: string[] = [] | ||
| private listeners: Map<string, Set<(...args: any[]) => void>> = new Map() | ||
| private handlers: Set<string> = new Set() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a Set fixes a subtle issue. With an array, it's possible that we could end up with duplicates channels in the array.
You can only have 1 handler per channel, but the handle method will add the channel to the array even if a handler already exists for the channel.
When we dispose, we may end up attempting to remove listeners for a channel multiple times.
I don't think this causes any problems in practice, but using Set prevents the situation from happening in the first place.
|
@alex8088 What is the status of this? We just found ourselves having to decide between writing workarounds or re-implementing this library ourselves. This feels like a very reasonable improvement. Is there any reason why it has gone stale? |
Description
The main process
IpcListenernow returns cleanup functions for itsonandhandlemethods which remove the registered listener or handler.Its
disposemethod now unsubscribes only listeners and handlers registered with this class instance.Closes #19
Additional context
I added
vitestand tests for the main process'sIpcListener.I'll add some notes to the code.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).