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

Component updating without computed property dep changing #1811

Closed
paulshen opened this issue Aug 8, 2020 · 8 comments · Fixed by #5912
Closed

Component updating without computed property dep changing #1811

paulshen opened this issue Aug 8, 2020 · 8 comments · Fixed by #5912
Labels
has PR A pull request has already been submitted to solve the issue scope: reactivity ✨ feature request New feature or request

Comments

@paulshen
Copy link

paulshen commented Aug 8, 2020

What problem does this feature solve?

I'm relatively new to Vue. Learning about computed properties (cool feature!), I expected a component that has a computed dependency to only update when the computed value "changes" (wrt hasChanged) instead of updating whenever the recursive deps change.

Here is a CodeSandbox example https://codesandbox.io/s/floral-pine-nvk0f?file=/src/App.vue
The watch does its own hasChanged check and updates only when the computedCount changes (10x less than count). I expect the component to update at the same rate.

A practical use case is ability to run a quicker timer update (say every 100ms) with the UI only rendering seconds (every 1s). Another example is only rendering only user.name but the underlying user has irrelevant changes.

You can create a separate component to add the change checking but it seems it could work in same component. I couldn't find a relevant issue. I can see the complexity tradeoff (needs external state when tracking read dependencies) but was wondering if that was the case.

What does the proposed API look like?

Same API but different component update behavior.

@posva
Copy link
Member

posva commented Aug 8, 2020

Note updating doesn't directly translate in HTML being patched.

@paulshen
Copy link
Author

paulshen commented Aug 8, 2020

Yep, but it still needs a VDOM and lifecycle pass. I'm not reporting this as a bug as I think it's reasonable if the answer is this optimization is delegated to the VDOM.

It seems possible to implement if you add some global state to track whether you're in the middle of derived data.

@yyx990803
Copy link
Member

This has to do with another optimization that prevents the computed getter from being over-eagerly evaluated too many times:

const expensiveComputed = computed(() => {
  return someArray.map(() => { ... })
})

for (let i = 0; i < 1000; i++) {
  someArray.push({ ... })
}

If computed getters are evaluated eagerly and synchronously, the above would result in the getter called 1000 times when in fact it needs to be called only once. So internally instead of calling the getter every time a dep changes, we simply mark the computed "dirty" and trigger effects that depend on it. The getter is then only called once, on demand, when the triggered effect accesses it.

However, this means we can't know the new value at the time of mutation before triggering the effects, so we can't compare it to the old value to avoid unnecessary triggers.

This is a trade-off that I don't know if is possible to "fix", but I think the current behavior is somewhat better because you can workaround it using a manual ref + watchEffect (basically a manual eager computed):

const computedCount = ref(0)
watchEffect(() => {
  computedCount.value = Math.floor(count.value / 10)
}, { flush: 'sync' })

@paulshen
Copy link
Author

Thanks for the reply @yyx990803! This makes sense

To avoid the case you describe, could the computed properties be marked dirty on dep set but updated on a tick, similar to reactive effects? Basically non-eager and async. If something reads before the delayed tick, it has the dirty flag. There is probably another tradeoff 😅

@Picknight
Copy link
Contributor

Picknight commented Aug 17, 2020

  1. I run the example provided by @paulshen in master branch 619efd9, the trigger frequency of watch is the same as that of onUpdated:
App.vue?3dfd:22 computedCount updated 0
App.vue?3dfd:27 updated
App.vue?3dfd:22 computedCount updated 0
App.vue?3dfd:27 updated
App.vue?3dfd:22 computedCount updated 0
App.vue?3dfd:27 updated
App.vue?3dfd:22 computedCount updated 0
App.vue?3dfd:27 updated
App.vue?3dfd:22 computedCount updated 0
App.vue?3dfd:27 updated
App.vue?3dfd:22 computedCount updated 0
App.vue?3dfd:27 updated
App.vue?3dfd:22 computedCount updated 0
App.vue?3dfd:27 updated
App.vue?3dfd:22 computedCount updated 0
App.vue?3dfd:27 updated
App.vue?3dfd:22 computedCount updated 0
App.vue?3dfd:27 updated
App.vue?3dfd:22 computedCount updated 1
App.vue?3dfd:27 updated
App.vue?3dfd:22 computedCount updated 1
App.vue?3dfd:27 updated
App.vue?3dfd:22 computedCount updated 1
App.vue?3dfd:27 updated
  1. I wrote a special case mentioned by @yyx990803 codesandbox, but in fact computed getters only triggered once, I am not sure if it is my problem.

@yyx990803
Copy link
Member

This is fixed by d05c9a7 (will be out in 3.2)

@johnsoncodehk
Copy link
Member

johnsoncodehk commented Aug 26, 2023

Reopen as this will be re-fixed in 3.4 by #5912.

@johnsoncodehk johnsoncodehk reopened this Aug 26, 2023
@pikax pikax added the has PR A pull request has already been submitted to solve the issue label Oct 21, 2023
yyx990803 pushed a commit that referenced this issue Oct 27, 2023
sxzz pushed a commit that referenced this issue Oct 27, 2023
@johnsoncodehk
Copy link
Member

Fixed by #5912.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
has PR A pull request has already been submitted to solve the issue scope: reactivity ✨ feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants