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

feat: add svelte/events package and export on function #11912

Merged
merged 15 commits into from
Jun 6, 2024

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Jun 5, 2024

This PR introduces the svelte/events package, which exports the on function. The function should be used in place of imperatively doing addEventListener, as it co-ordinates the event handling with Svelte's internal event delegation system.

Copy link

changeset-bot bot commented Jun 5, 2024

🦋 Changeset detected

Latest commit: 38dbd78

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

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

* @param {AddEventListenerOptions} [options]
*/
export function attach(dom, event_name, handler, options = {}) {
var target_handler = create_event(event_name, dom, handler, options);
Copy link
Member

Choose a reason for hiding this comment

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

create_event calls addEventListener, it's not using event delegation. What am I missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It uses the target function which ensures any delegated events run as expected, whilst still attaching the event to the target manually.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. The documentation is a bit misleading in that case, will rewrite it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

@trueadm trueadm changed the title feat: add svelte/events package and export attach function feat: add svelte/events package and export on function Jun 5, 2024
@david-plugge
Copy link

Wait so we should not use addEventListener at all now? What about non svelte libraries?

@dummdidumm
Copy link
Member

This is only relevant in edge cases where you rely on the elements getting invoked in the right order or when calling stopPropagation. When the code is under your control, better safe than sorry to use this new function. Outside Svelte libraries very likely either don't have that reliance or are self-contained, so its a non-issue.

@david-plugge
Copy link

Thanks for clarification!

@Rich-Harris Rich-Harris merged commit aadf629 into main Jun 6, 2024
8 checks passed
@Rich-Harris Rich-Harris deleted the expose-events-attach branch June 6, 2024 19:25
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.

None yet

5 participants