Skip to content

Conversation

@justsl
Copy link
Contributor

@justsl justsl commented Dec 13, 2024

Changes in PR:

Add remove event listener from emitter on unmounted.


if (import.meta.env.DEV) {
expose({
list,
Copy link

Choose a reason for hiding this comment

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

Code Review:

  1. Bug Risk:

    • No obvious bug risks related to the code changes in this patch.
  2. Improvement suggestions:

    • By adding the onUnmounted lifecycle hook, you are correctly cleaning up event listeners. This is good practice and helps prevent memory leaks.
    • Consider organizing your imports more efficiently. You might want to group them based on where they are coming from or their type to make them easier to read and manage.
  3. General Comments:

    • The addition of the onUnmounted hook improves the cleanup process, ensuring that event listeners are removed when the component is unmounted, which is a good practice.
    • It's important to maintain consistency in code style and formatting. Ensure that you follow the existing conventions in the codebase to promote readability and maintainability.

Overall, the code changes look appropriate as they address cleanup concerns related to event listeners effectively by incorporating the onUnmounted hook.

@kyvg kyvg merged commit 73107fa into kyvg:master Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants