QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.83k stars 1.31k forks source link

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

Closed jwickers closed 2 weeks ago

jwickers commented 1 year ago

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

stackblitz[bot] commented 1 year ago

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

shairez commented 5 months ago

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

wmertens commented 2 weeks ago

fixed in v2 https://qwikdev-build-v2.qwik-8nx.pages.dev/playground/#f=7Vjfa9swEH7fX6GFUbs0mI2NPZg1ozSBddvDoGPdHh1Ha0s8K1hOQgj53%2FedTpblH11LWUtL%2BxJi%2B3T33en06e68pHn77s17t1MQeDXET50G9NTIXvuCL2b74HPf8MUN0o3D8x3cXHPyYX%2FysWiGpNflF7mBmLMeBgHcqUTmcnNsrglf4rUnYBiqX6TrRhhu7UW%2Fc1CcJaw33yxMBy4COyylMcmyVR0SNFyN4xIm4th3KhiKuVlIR6oVck6XwOAPKOTYpTCUHi5Yc84xiIMDRlHjWCdF3gFidYoK9G6fc6QfAUIMysodBib4UK4g0kBT7UUfllJI%2BGvW4IYQcH%2BqkmI2oRc3wuxQONSivQVkIErVTDa9uvIEbtvr9%2FY6Kl9ikyY6TRYyEB%2BtgnahzQcS8DNVHA7OCynzwehMFXONk9gQai2iZaOvxiJFJO4g2rUVVEX7PzWaEpWraahs58jtdFK%2BVhqb%2B3yNvsbjvog7EW5E1Vf0Sy2pCNKofMTk9Pjo20REUYRKFtUV6geTSxoXscjQPKAAWCPiEUo4ct7g2wYi2HkKu6GgqLNsxyvPAR%2B9jxXFCwoiqqyx1hPjtTXzUg5eTYFnBUgZLj4dJrQePxhCdHgeHy866P%2BRHr3zejcMWcjZYDRWS%2BQac8ozTd45TSZoDjE3YHBmRoHOMS2zjWVMT6Hlw3smy3oa5fPlLSdfizpAnyQ8R8uZeBOG8iIBpWI2INGDlMo4VsUJYTOe6yE6SnTzCOFYrpw69GDmLNJmG5do0IWw0%2B6KNAFPkKnp8txoovGEu5%2FQw6WwAHuIglPIHbQqKCwWROTqeOfHh2bVXJf6PZ4SZE0TgdphjPGARYvPpz9JYm2vHDT3iRif%2FBia1GDjTh1lDMcIkgANh8v1ZSrJ04jbeAprphWmIebiIH1k%2FCTXC%2BQWfEpqdVOFuQR9NcYpajDsspT%2BX%2BYa3S7F7CLBDMQlLx7zGTp8Fxchjn5j1MVZb%2BPPXvO0zCijYcoftQJ2APQOQXUAVJ7Vm6B4AlUdieviX13ZdhvayfvcGz%2B%2B3vgv

wmertens commented 2 weeks ago

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