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

Reactive fail on 3.4.13 #10114

Closed
mrquokka opened this issue Jan 15, 2024 · 11 comments · Fixed by #10119 or #10123
Closed

Reactive fail on 3.4.13 #10114

mrquokka opened this issue Jan 15, 2024 · 11 comments · Fixed by #10119 or #10123
Labels
❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. regression scope: reactivity

Comments

@yyx990803 yyx990803 added regression ❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. scope: reactivity labels Jan 15, 2024
@yyx990803
Copy link
Member

/cc @johnsoncodehk

@LinusBorg
Copy link
Member

LinusBorg commented Jan 15, 2024

@yyx990803 Is that problem reproducible without a side-effect in the computed (or in render)? I can't find a way, at least.

If it's not, then as a theoretical question: Should we really support that? Or is it that we don't want to but need to support it to keep backwards compat for something that worked even though it has always been a discouraged practice?

@yyx990803
Copy link
Member

This is indeed discouraged usage, but this is a regression nonetheless and should ideally be fixed.

@Doctor-wu

This comment was marked as resolved.

@yyx990803

This comment was marked as resolved.

@Doctor-wu

This comment was marked as resolved.

@yyx990803

This comment was marked as resolved.

@Doctor-wu

This comment was marked as resolved.

@Doctor-wu
Copy link
Contributor

I think the key to this problem is the computed was mutated in render lead the computed was marked to Dirty after the invocation of the render function. And I think that is a dangerous state of computed (marked as dirty, which means it won't be reactive anymore)

@mrquokka
Copy link
Author

mrquokka commented Jan 16, 2024

@yyx990803 Is that problem reproducible without a side-effect in the computed (or in render)? I can't find a way, at least.

If it's not, then as a theoretical question: Should we really support that? Or is it that we don't want to but need to support it to keep backwards compat for something that worked even though it has always been a discouraged practice?

@LinusBorg , hi, it was a part of some old js code with chartgpt translation to ts (with some bad practics). Can you make another short example how we can make it perfect?
We use vuex to lazy-load translations from server.
On this component we send some keys for translation.
We need make prop with keys reactive.

Of course, we can use watch with immediate and make computed watch keys (array is simple example, on project it may by dict) => and it will get soooo long and complex structure with hard upgrades after 2+ years if files not changes

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. regression scope: reactivity
Projects
None yet
4 participants