Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

[🐞] V2: Bug when signal of useComputed$ is used in useTask$ or useVisibleTask$ #7213

Open
fabian-hiller opened this issue Dec 30, 2024 · 5 comments · May be fixed by #7435
Open

[🐞] V2: Bug when signal of useComputed$ is used in useTask$ or useVisibleTask$ #7213

fabian-hiller opened this issue Dec 30, 2024 · 5 comments · May be fixed by #7435
Assignees
@Varixo
Labels
COMP: runtime TYPE: bug Something isn't working VERSION: upcoming major

Comments

@fabian-hiller
Copy link
Contributor

fabian-hiller commented Dec 30, 2024 •
edited
Loading

Which component is affected?

Qwik Runtime

Describe the bug

When a useComputed$ signal is read in useVisibleTask$, the task is executed twice (or multiple times if multiple useComputed$ are used). Also, useComputed$ is unnecessarily re-executed afterwards.

Reproduction

https://stackblitz.com/edit/github-o2f6fj9n-meq2plo7?file=src%2Froutes%2Findex.tsx

Steps to reproduce

  1. Open StackBlitz link
  2. Open and clear browser console
  3. Reload Qwik App

Now you should see this in the console:

useVisibleTask$: Do stuff with text...
useVisibleTask$: Do stuff with text...
useComputed$: Computing initial text...

The expected output would be:

useVisibleTask$: Do stuff with text...

This is because useComputed$ should only run once on the sever and useVisibleTask$ should only run once in the browser. A similar bug exists when using useTask$.

System Info

-

Additional Information

No response
@fabian-hiller fabian-hiller added STATUS-1: needs triage New issue which needs to be triaged TYPE: bug Something isn't working labels Dec 30, 2024
@Varixo
Copy link
Member

Varixo commented Dec 31, 2024

https://qwikdev-build-v2.qwik-8nx.pages.dev/playground/#f=7VZBDoIwELz7it7gYDEmhkQinnyCX4CEC5oAXggv8OQL%2FKJPcHa30AUNiTHevBDabrft7OzuKNLEm3gIFNY9AZYYSfulP81nN9YVb7n4TroIXsRp4DXKHW8jVn1rD7RVYuSXtEZRFjXSkZ1FURTgxbTXQR%2FgRsYQb2fCAbVUmMf9dvUiCdNHtBL2mrb07XqYPbDklblNpsjZc4XX0NXJPnFo7kQTUfZM%2BcInpAohPpMd7cOwFXy6ARC01KyC4qit6%2FC2hIHFTMYyaVWeLFCyFwmUrRGp19CFUqbncNbmiTlAjNRNnkv%2Fm8Cst%2FISCmaT8eIYdBaY9DyRmIze%2B1Ky3iqx%2FEkpGTHxX0h%2BUUie

@Varixo Varixo added COMP: runtime VERSION: upcoming major and removed STATUS-1: needs triage New issue which needs to be triaged labels Dec 31, 2024
@Varixo Varixo self-assigned this Dec 31, 2024
@Varixo Varixo moved this to In progress in Qwik Development Dec 31, 2024
@Varixo
Copy link
Member

Varixo commented Jan 11, 2025

Analysis:
in v2 when the QRL is not resolved at the time when it is needed we throw a promise and retry when the promise is resolved. The text.value was not needed before, so it will cause to throw and rerun the task.

Expected output:

 useVisibleTask$: Do stuff with text...
 useComputed$: Computing initial text...

The computed should execute on client, because in v2 it is lazy and on the server it was not needed.

Workaround:

  useVisibleTask$(async () => {
    console.log(text.value); // <-- read the computed value first
    console.log('useVisibleTask$: Do stuff with text...'); <-- then execute the rest
  });

We should probably think how to solve this anyway

@fabian-hiller
Copy link
Contributor Author

Thanks Michał! There is a similar "bug" with useTask$. Does the same apply here?

@Varixo
Copy link
Member

Varixo commented Jan 11, 2025

hmm I think useTask$ is working correctly, right?
https://qwikdev-build-v2.qwik-8nx.pages.dev/playground/#f=7VZBDoIwELz7it7gYDHGxEQinnyCX4CEC5pQvBBe4MkX%2BEWf4OxuoQsxJMZ480Jou912p7s7o5JmuxnCoRuGBFhiJPRLfzqf%2FZhb3XLxnWYRoCiZAdSoaIKNWPWcHmmr1MgviYyyKh3qkJ0lSRIhVNrrMY9wI2MoYWfeATKpNM%2FH%2FRbUEaZP4BD2mrX07Xp8A6LklZOaTFGslxrR0NXJPvUw7kUMUdlME4VPyBRCfCY7OsRxK%2Fh0AyDg0ryG1HDWU7utYGAxk7M%2BWlVnC5TsVTjJOryUerNYGvMcwJrNUnOE%2FHBNUQjjTfDVW3kJLbLJeXGMNktKiktEJcP2vnmsd0oef9I8Rin4bx2%2FaB0v

obraz

@fabian-hiller
Copy link
Contributor Author

Yes, I think you are right. My bad.

@Varixo Varixo linked a pull request Mar 17, 2025 that will close this issue
fix: early schedule computed qrls to resolve #7435
Draft
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@Varixo Varixo

Labels
COMP: runtime TYPE: bug Something isn't working VERSION: upcoming major
Projects
Qwik Development
Status: In progress
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: early schedule computed qrls to resolve QwikDev/qwik
2 participants
@fabian-hiller @Varixo