-
-
Notifications
You must be signed in to change notification settings - Fork 174
Description
Description
Event listeners registered for plugin events are not reliably removed during component updates because different function references are passed to pm.on() and pm.off().
This causes listeners to accumulate over time, resulting in duplicated event emissions and potential memory leaks when the component re-renders or updates.
Expected result
Event listeners should be removed using the exact same handler reference that was used during registration, ensuring:
- no duplicated events
- proper cleanup during updates and unmount
- stable listener lifecycle
Actual result
private setupEventListeners() {
const { onPluginEvent } = this.props;
const { pm } = this.state;
if (!onPluginEvent) return;
PLUGINEVENTS.forEach((event) => {
pm?.on(event, this.handler(event));
});
}
private cleanupEventListeners() {
const { pm } = this.state;
PLUGINEVENTS.forEach((event) => {
pm?.off(event, this.handler(event));
});
}
Right now the Listeners are registered and cleaned up using:
pm.on(event, this.handler(event));
pm.off(event, this.handler(event));
Since this.handler(event) creates a new function instance on every call, off() does not receive the same reference that was originally registered with on().
Proposed solution
Maintain stable handler references for each plugin event.
- Create handlers once (for example in the constructor).
- Store them in a
Map<string, Function>keyed by event name. - Reuse the stored handler for both
pm.on()andpm.off().
private eventHandlers = new Map<string, (data: unknown) => void>();
PLUGINEVENTS.forEach((event) => {
this.eventHandlers.set(event, (data) => {
this.props.onPluginEvent?.(event, data);
});
});
and then,
pm.on(event, handler);
pm.off(event, handler);
Steps to reproduce
- Create a plugin that periodically emits a plugin event.
- Render
AsyncApiComponentWPwith that plugin and anonPluginEventcallback. - Change the
onPluginEventhandler identity (for example, by updating state via a button click). - Observe the console output (each emission logged many times)
Additional context
- This issue is not limited to
onPluginEventidentity changes. It can also occur when listener setup/cleanup runs with new handler references or when the plugin manager instance changes, leaving listeners attached to the previous instance. - Listeners should be rebound if the plugin manager instance changes (
prevState.pm !== pm) to avoid leaving listeners attached to an old instance.
Happy to open a PR for this if the maintainers approve.