Skip to content
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

fix: properly remove event listeners in Component's removeEventListener (v4) #13556

Open
wants to merge 1 commit into
base: svelte-4
Choose a base branch
from

Conversation

ovx
Copy link

@ovx ovx commented Oct 10, 2024

  • fix: properly remove event listeners in Component's removeEventListener

Copy link

changeset-bot bot commented Oct 10, 2024

⚠️ No Changeset found

Latest commit: e6d428e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ovx ovx changed the base branch from main to svelte-4 October 10, 2024 13:03
@ovx
Copy link
Author

ovx commented Oct 10, 2024

The event listeners are not properly removed when using removeEventListener for custom events. The method addEventListener adds a listener to the internal this.$$l map but the method removeEventListener does not remove them.

The issue is painfully obvious when using a WebComponent (built form a Svelte component) in React with strict mode enabled, where the component is mounted twice resulting in the listener being attached twice without the first one being removed.

Example:

const button = document.querySelector('my-button');

const handler = () => {
  console.log('myaction handler');
};

// add and remove, simulating React's strict mode
button.addEventListener('myaction', handler);
button.removeEventListener('myaction', handler);

// only add
button.addEventListener('myaction', handler);

The code above does not remove the first listener, firing the handler twice.

@ovx ovx closed this Oct 10, 2024
@ovx ovx reopened this Oct 10, 2024
@ovx ovx changed the title svelte 4 fix: properly remove event listeners in Component's removeEventListener (v4) Oct 10, 2024
@dominikg
Copy link
Member

dominikg commented Oct 14, 2024

@ovx

it looks like you published copies of svelte and vite-plugin-svelte to the public npm registry, keeping the original metadata for repository and license etc. in place. This can be confusing for both users and tools (eg github now shows your plugin as first if you look up the dependents of vite-plugin-svelte https://github.com/sveltejs/vite-plugin-svelte/network/dependents)

Please remove/make them private or update all metadata to make it clear they are not provided by the svelte org but forks.

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.

CSS Adjacent Sibling Combinator (+) is causing some CSS to be erroneously removed
2 participants