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

[🐞] Event handlers either disappearing or included twice depending on the component JSX #4464

Closed
jwickers opened this issue Jun 12, 2023 · 4 comments
Labels

Comments

@jwickers
Copy link
Contributor

Which component is affected?

Qwik Runtime

Describe the bug

There is a weird interaction between component event handlers like useOn, useOnDocument or useOnWindow and the JSX structure.

Reproduction

You can see this stackblitz as well: https://stackblitz.com/edit/qwik-starter-mkqcen?file=src%2Froutes%2Findex.tsx

Press any key on the page to start and see both components render the events counts.
Press the escape key to switch the component render which changes the behavior.

BUG 1

When the component renders fragment, Qwik places a <script type="placeholder" tag with the Qwik event handlers (like on-document:xxx). If the component changes to render a DIV for example, that placeholder is removed and the DIV now has the event handlers. So far so good.
But if the component renders back to a fragment then the DIV is not replaced back with a placeholder tag thus losing the event handlers altogether.

BUG 2

An easy workaround seems to be to avoid using a fragment as root in the JSX and say wrap the whole component in a DIV.
But when the component changes to render an inner DIV, that inner DIV then also gets the event handlers, causing those events to be processed twice.

Reproduction

https://github.com/jwickers/qwik-useon-events-bug

Steps to reproduce

Run npm install and npm run start.

System Info

System:
    OS: macOS 13.3.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
    Memory: 36.66 GB / 64.00 GB
    Shell: 5.9 - /usr/local/bin/zsh
  Binaries:
    Node: 18.16.0 - ~/.nvm/versions/node/v18.16.0/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.6.7 - ~/.nvm/versions/node/v18.16.0/bin/npm
  Browsers:
    Brave Browser: 114.1.52.122
    Chrome: 114.0.5735.106
    Edge: 114.0.1823.37
    Firefox: 92.0
    Firefox Developer Edition: 109.0
    Safari: 16.4
  npmPackages:
    @builder.io/qwik: ^1.1.5 => 1.1.5
    @builder.io/qwik-city: ^1.1.5 => 1.1.5
    undici: 5.22.1 => 5.22.1
    vite: 4.3.9 => 4.3.9

Additional Information

No response

@jwickers jwickers added TYPE: bug Something isn't working STATUS-1: needs triage New issue which needs to be triaged labels Jun 12, 2023
@stackblitz
Copy link

stackblitz bot commented Jun 12, 2023

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

@shairez
Copy link
Contributor

shairez commented May 25, 2024

Thanks a lot @jwickers and sorry for the delay

If this isn't fixed by now, it'll definitely be easier to solve in V2 I believe

We'll add a test for it in the V2 branch to verify this is not repeating

@shairez shairez added VERSION: upcoming major STATUS-2: team is working on this Scheduled for work by the core team and removed STATUS-1: needs triage New issue which needs to be triaged labels May 25, 2024
@maiieul maiieul removed STATUS-2: team is working on this Scheduled for work by the core team P2: minor labels Aug 17, 2024
@wmertens wmertens moved this from Backlog to Waiting For Review in Qwik Development Nov 1, 2024
@wmertens
Copy link
Member

wmertens commented Nov 2, 2024

I'm closing this since it's fixed in v2. We're hoping to get the alpha out soon.

@wmertens wmertens closed this as completed Nov 2, 2024
@github-project-automation github-project-automation bot moved this from Waiting For Review to Done in Qwik Development Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

4 participants