-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat(typed-ipc): main process IpcListener
cleanup functions
#20
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:
dispose
removes all listeners for each channel, even those which were not registered by the instance. - New behavior:
dispose
removes 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.
Description
The main process
IpcListener
now returns cleanup functions for itson
andhandle
methods which remove the registered listener or handler.Its
dispose
method now unsubscribes only listeners and handlers registered with this class instance.Closes #19
Additional context
I added
vitest
and 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
).