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

[🐞] Signals don't trigger rerender with some problematic JSX markup #5782

Closed
EatChangmyeong opened this issue Jan 27, 2024 · 5 comments
Closed
Assignees
Labels
STATUS-3: solved by V2 This issue is solved by the next major version of Qwik

Comments

@EatChangmyeong
Copy link

Which component is affected?

Qwik Runtime

Describe the bug

I was randomly writing some edge case code (orphaned signals in this case) to see what happens when I first encountered this bug. Trying to reproduce the bug in the playground later revealed that this bug depends on the returned JSX markup.

In the following reproduction, app.tsx exports two components V1 and V2, which have different JSX markup (V1's children are nested in extra p's) but are otherwise the same.

Expected behavior: apart from the visual differences (from not having extra p's), both components should show the same behavior, that is:

  • clicking on ++ and -- buttons should always increment and decrement the counter (both parent's and child's), and
  • the counter should reset to zero whenever Child rerendering triggers its useTask$.

Actual behavior: Once V2's Child unmounts for the first time, the parent counter gets stuck and no longer updates. V1 does not show this behavior and works as expected.

Reproduction

https://qwik.builder.io/playground/#v=1.4.2&f=5Va9DoIwEN55ClwICWAENgKdfATD7oA4kGA0xoH03b3eHW1BEkEdSFz4aa%2FXu%2B%2B7PztoklT7Y0dNaNovfmJhY%2BpoeU5cEQ5lDChMhhbtY3mvrnumrjBX%2BzswuBeDijnYGx6zBc%2Ftg0q3LX46NrcKpfqAZmAuBkscckxv78hSuHgLiX%2Bv6BlFUkSRHon4cDeSksuUBoEUQTBUqpJsrom9y6QNHN%2BMlqQ4tHXdVK92j456npsTemBiodySEFHkDlsEsUTJP%2BI5WTXPn7M7ze03zP6YxgUkMneOQxj2%2BNm0dUiBzJA6%2BMw443N%2B0YwnBKiZJHiCWT1yg3IaoM1Zl1Z8uAll8U4NAOtUGyri7HbU8R4j8789KYlT41kZhyoTjTVraU3QBnRnUr%2FJ%2B0b1BA

Steps to reproduce

No response

System Info

System:
    OS: Linux 4.14 Arch Linux ARM
    CPU: (8) arm64 Cortex-A55
    Memory: 860.85 MB / 3.58 GB
    Container: Yes
    Shell: 5.2.21 - /bin/bash
  Binaries:
    Node: 20.10.0 - /usr/sbin/node
    npm: 10.3.0 - /usr/sbin/npm
  npmPackages:
    @builder.io/qwik: ^1.3.5 => 1.4.2
    @builder.io/qwik-city: ^1.3.5 => 1.4.2
    undici: * => 6.5.0
    vite: ^5.0.11 => 5.0.12

Additional Information

No response

@EatChangmyeong EatChangmyeong added STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working labels Jan 27, 2024
@wmertens
Copy link
Member

Thank you for the repro, that indeed looks like a bug.

@wmertens wmertens added COMP: runtime P2: important and removed STATUS-1: needs triage New issue which needs to be triaged labels Jan 27, 2024
@JerryWu1234
Copy link
Contributor

Let me take a look.
It would be great if you could offer a clue that off the top of your head about this bug.

@wmertens wmertens added the WAITING FOR: team Waiting for one of the core team members to review and reply label Feb 27, 2024
@wmertens
Copy link
Member

Since we have v2 well underway and it rebuilds some of this code, let's wait until we can test this with v2 alpha.

@aarbi
Copy link

aarbi commented Mar 4, 2024

Also I want to point out another related issue with the example app we get after running pnpm create qwik and choosing the Basic App starter, the counter there is broken and can reproduced quickly by running pnpm preview, hitting the "+" button makes the number drops from 70 to 50

Varixo added a commit to Varixo/qwik that referenced this issue Mar 7, 2024
mhevery added a commit that referenced this issue Mar 9, 2024
* test(v2): test for issue #5782

* fixup

* fixup

---------

Co-authored-by: Miško Hevery <[email protected]>
@maiieul maiieul added VERSION: after next major Features we'd want to add after the next version is done and removed WAITING FOR: team Waiting for one of the core team members to review and reply labels Aug 17, 2024
@gioboa gioboa self-assigned this Nov 27, 2024
@gioboa gioboa added STATUS-3: solved by V2 This issue is solved by the next major version of Qwik and removed TYPE: bug Something isn't working COMP: runtime VERSION: after next major Features we'd want to add after the next version is done labels Nov 27, 2024
@gioboa
Copy link
Member

gioboa commented Nov 27, 2024

I did a double check with the latest v2 code and it's working fine.
v2 will be release soon so I marked this issue with STATUS-3: solved by V2, thanks.

@gioboa gioboa closed this as completed Nov 27, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in Qwik Development Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
STATUS-3: solved by V2 This issue is solved by the next major version of Qwik
Projects
Status: Done
Development

No branches or pull requests

6 participants