Skip to content

fix: cleans up plugin event listeners on update and unmount properly#1272

Open
Ishita-190 wants to merge 21 commits into
asyncapi:masterfrom
Ishita-190:ishi
Open

fix: cleans up plugin event listeners on update and unmount properly#1272
Ishita-190 wants to merge 21 commits into
asyncapi:masterfrom
Ishita-190:ishi

Conversation

@Ishita-190
Copy link
Copy Markdown

Description

Changes proposed in this pull request:

  • created a handler function for each event, store it in the eventHandlers Map which will then be reused for both pm.on and pm.off
  • Added componentWillUnmount to AsyncApiComponent for proper event listener cleanup.

verified this using a test, it's working properly :

Screenshot 2026-04-27 002536

Related issue(s)

Fixes #1235

Copy link
Copy Markdown
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

Hey @Ishita-190

Can you fix your failing tests? Once done, i'll give it a review

Copy link
Copy Markdown
Member

@AceTheCreator AceTheCreator left a comment

Choose a reason for hiding this comment

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

Left you a review :)

}

class AsyncApiComponent extends Component<AsyncApiProps, AsyncAPIState> {
componentWillUnmount() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I find the lifecycle placement hard to read. I don't think componentWillUnmount should be at the top-level

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sir, I've placed it below componentDidUpdate, I hope this fixes the issue.

this.props.onPluginEvent!(eventName, data);
};
}
private eventHandlers = new Map<string, (data: unknown) => void>();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Make this readonly

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done

AceTheCreator and others added 16 commits May 2, 2026 20:24
* fix: add node_auth for npm release fallback

* fix: indentation
* fix: add node_auth for npm release fallback

* fix: indentation

* chore(fix): try token as fallback
* fix: add node_auth for npm release fallback

* fix: indentation

* chore(fix): try token as fallback

* fix: bump node engine  > 20
…1232)

Co-authored-by: Ahmadshah Donishyar <ahmadshahdonishyar@gmail.com>
Co-authored-by: Azeez Elegbede <40604284+AceTheCreator@users.noreply.github.com>
Co-authored-by: Mohammed Mehdi <96487647+catosaurusrex2003@users.noreply.github.com>
@sonarqubecloud
Copy link
Copy Markdown

@Ishita-190 Ishita-190 requested a review from AceTheCreator May 21, 2026 08:19
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

4 participants