Skip to content

fix(react): cache event handler references for proper cleanup (#1235)#1265

Open
armorbreak001 wants to merge 1 commit intoasyncapi:masterfrom
armorbreak001:fix/event-listener-cleanup
Open

fix(react): cache event handler references for proper cleanup (#1235)#1265
armorbreak001 wants to merge 1 commit intoasyncapi:masterfrom
armorbreak001:fix/event-listener-cleanup

Conversation

@armorbreak001
Copy link
Copy Markdown

Problem

Fixes #1235

Event listeners registered via pm.on() are never properly removed by pm.off() because each call to setupEventListeners() creates new function references.

Root Cause

// BEFORE (broken):
private handler(eventName: string) {
  return (data: unknown) => {   // ← new function every call!
    this.props.onPluginEvent!(eventName, data);
  };
}

// setupEventListeners calls: pm.on('event', this.handler('event')) → fn#1
// cleanupEventListeners calls: pm.off('event', this.handler('event')) → fn#2
// fn#2 !== fn#1, so pm.off() cannot find and remove the listener

Impact

  • Duplicate events: Every component re-render adds more listeners
  • Memory leak: Old listeners are never garbage collected
  • pm.off() is a no-op: Cleanup code appears to work but does nothing

Fix

Cache handler functions in a Map<string, Function> so the same reference is always used:

// AFTER (fixed):
private readonly eventHandlers = new Map<string, (data: unknown) => void>();

private getEventHandler(eventName: string): (data: unknown) => void {
  if (!this.eventHandlers.has(eventName)) {
    this.eventHandlers.set(eventName, (data: unknown) => {
      this.props.onPluginEvent!(eventName, data);
    });
  }
  return this.eventHandlers.get(eventName)!;
}

Now pm.on(event, getEventHandler(event)) and pm.off(event, getEventHandler(event)) receive identical function references.

Testing

  • ✅ All 177 unit tests pass
  • ✅ No behavior change — same events fired with same data
  • ✅ Handlers cached per event name for component lifetime

…pi#1235)

Problem: setupEventListeners() creates new function references on every
call via , so cleanupEventListeners() passes
different references to pm.off() — listeners are never removed.

This causes:
- Duplicate event emissions on re-render
- Memory leaks from accumulating listeners
- pm.off() is effectively a no-op

Fix: Cache event handlers in a Map<string, Function> so that
pm.on() and pm.off() always receive the same reference for the
same event name.

Before: handler(event) → new function each call (off can't match)
After:  getEventHandler(event) → cached function (off matches exactly)

All 177 unit tests pass.
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.

@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Event listeners are not properly cleaned up in AsyncApiComponent

1 participant