Skip to content

Svelte 5: onkeydown event triggers more often than it should #10271

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.

Sign up for GitHub

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

Jump to bottom
Closed
r-unruh opened this issue Jan 23, 2024 · 2 comments · Fixed by #10307 or #10611
Closed

Svelte 5: onkeydown event triggers more often than it should #10271

r-unruh opened this issue Jan 23, 2024 · 2 comments · Fixed by #10307 or #10611
Assignees
@dummdidumm
Labels
bug
Milestone
5.0

Comments

@r-unruh
Copy link

r-unruh commented Jan 23, 2024

Describe the bug

<svelte:window /> seems to produce some odd behavior in components. See reproduction.

I'm pretty sure I didn't have this issue in Svelte 5 version 34. It started with 35.

(Link to Discord thread)

Reproduction

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAA62QQWuDQBCF_8oyFxMQvVsjlEJpDz3n0O1BdEyXuDPLOsYG8b-XVWhq00MoPe7sN--9eSM0psUOstcRqLQIGdw7BzHI2YVHd8JWEGLouPdVmORd5Y2TQpMWYx17UY_s7QNbx4QkqvFsVZSkq2myCEV3msJi01Mlhkm9l1S3uDdU87DZqjF8aqmYOm4xafmwiZ4CYuigXkpDakGjbRCSSVOeXvJQvrhkwwwppiOeax5oN373mVRaaMrXqdMCYrBcm8ZgDZn4Hqf4q5MVe2s7fzrykujnpVeCz-R6uV1vxv-hOE25CVIqlLDTIPghGq53Zr_pl1rfpk8h3uptdgIAAA==

When you enter a character into the input field I expect three keyboard events to be triggered:

  • Main Window
  • Component Window
  • Component Input

However, Component Input is triggered twice, so there are four events in total.

Logs

No errors

System Info

System:
    OS: Linux 6.7 Arch Linux
    CPU: (4) x64 Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz
    Memory: 4.95 GB / 7.64 GB
    Container: Yes
    Shell: 5.9 - /usr/bin/zsh
  Binaries:
    Node: 21.6.0 - /usr/bin/node
    Yarn: 1.22.21 - /usr/bin/yarn
    npm: 10.3.0 - /usr/bin/npm
    pnpm: 8.14.0 - /usr/bin/pnpm
  Browsers:
    Chromium: 121.0.6167.71
  npmPackages:
    svelte: 5.0.0-next.37 => 5.0.0-next.37

Severity

blocking all usage of svelte
@dummdidumm dummdidumm added the bug label Jan 25, 2024
@dummdidumm dummdidumm self-assigned this Jan 25, 2024
@dummdidumm
Copy link
Member

Analysis: Happens when adding the same event listener multiple times to the same the event, because handle_event_propagation is invoked for each event separately and the logic inside that function doesn't bail in that case, because of the "equal or lower means same event was re-dispatched again" logic.

dummdidumm added a commit that referenced this issue Jan 26, 2024
@dummdidumm
fix: handle event delegation correctly when having sibling event list…
febf62d 
…eners

If you had `on:` directives listening to the same name (through multiple on:click on the same element or indirectly through multiple `<svelte:window>` elements with event listeners of the same name) there was a bug of delegation firing too often. This PR fixes that by tweaking the "should I continue with the given path index" logic.
fixes #10271
@dummdidumm dummdidumm mentioned this issue Jan 26, 2024
Merged
5 tasks
dummdidumm added a commit that referenced this issue Jan 29, 2024
@dummdidumm
fix: handle event delegation correctly when having sibling event list…
76a4bbd 
…eners (#10307)

If you had `on:` directives listening to the same name (through multiple on:click on the same element or indirectly through multiple `<svelte:window>` elements with event listeners of the same name) there was a bug of delegation firing too often. This PR fixes that by tweaking the "should I continue with the given path index" logic.
fixes #10271
trueadm pushed a commit that referenced this issue Jan 31, 2024
@dummdidumm @trueadm
fix: handle event delegation correctly when having sibling event list…
bac8732 
…eners (#10307)

If you had `on:` directives listening to the same name (through multiple on:click on the same element or indirectly through multiple `<svelte:window>` elements with event listeners of the same name) there was a bug of delegation firing too often. This PR fixes that by tweaking the "should I continue with the given path index" logic.
fixes #10271
@r-unruh
Copy link
Author

r-unruh commented Feb 8, 2024

Is the fix included in v5.0.0-next.50 already? So far, the behavior is still the same.

@dummdidumm Are you seeing this?

@dummdidumm dummdidumm reopened this Feb 8, 2024
@dummdidumm dummdidumm added this to the 5.0 milestone Feb 8, 2024
@xxkl1 xxkl1 mentioned this issue Feb 22, 2024
Merged
5 tasks
dummdidumm pushed a commit that referenced this issue Feb 23, 2024
@xxkl1
fix: prevent window listeners from triggering events twice (#10611)
fc6666b 
fixes #10271
The prior fix in #10307 wasn't quite right, because JSDom has a difference between window and event.composedPath which sounds like a bug, and a workaround in that PR made it so the test did pass when the bug still occurred. That's also why there isn't a test here, because we can't properly test this using JSDom.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dummdidumm dummdidumm

Labels
bug
Projects
None yet
Milestone
5.0
2 participants
@dummdidumm @r-unruh