Passing the previous value to computed()

[skirtles-code]

This feature has already been merged into the minor branch of Vue core and seems likely to land in 3.4.0. But I thought it might still be useful to have a place to discuss it.

The PR is here: vuejs/core#9497.

When the callback for computed() is called, it will now be passed the old value. That's the value it returned the last time it was run.

The use case is for when the computed() returns an object. If the computed() returns the same object, it won't trigger downstream dependencies.

Some quick observations about the feature:

• I understand why this feature would be useful. It's something I've wanted myself on a few occasions.
• Adding this feature to Vue's core required very little code.
• It is also worth noting that this could be implemented in userland instead, e.g. in VueUse. It isn't something that has to be in Vue itself.

---

I do have concerns about this feature. I think it is likely to be misused.

The main problem will be that there are very tight restrictions on the legitimate use cases for the old value. Essentially, you are only allowed to use it to perform comparisons and then, conditionally, return it. If you deviate from that then things get very tricky, and I'm not sure most users will understand the nuances of what is allowed and why.

Documentation will obviously help, but I think this will be a difficult feature to document clearly. The restrictions on its use will probably come across as very arbitrary.

I also suspect that a lot of users will encounter this new feature by accident, maybe when their IDE tells them that their computed() is passed the oldValue. I doubt many people will go rushing off to the docs to study the caveats associated with using that oldValue, it appears self-explanatory what it would be.

Below I've included an example of the kinds of problems that can occur. This is split across 3 playgrounds:

• Playground 1 - This works 'correctly', though it is misusing the oldValue.
• Playground 2 - This appears to be the same as Playground 1, but with a minor refactoring to introduce a child component. It doesn't update correctly when the Count value is changed.
• Playground 3 - This introduces a chained computed() to Playground 1, which also breaks it.

The problematic computed() in these three examples look like this:

const basket = computed((oldValue) => {
  if (oldValue && oldValue.id === product.value) {
    oldValue.count = count.value
    return oldValue
  }

  return {
    id: product.value,
    count: count.value
  }
})

The problems seen here are similar to the problems encountered when using a shallowRef(). However, with shallowRef() there is an expectation that it is an advanced feature with lots of pitfalls. This change has introduced those pitfalls to a much more fundamental, common-place part of the API.

There are various other ways to misuse oldValue, but what I find interesting about this example is that it seems very similar to the legitimate use cases. It also seems to work in the first example. It's not necessarily 'obvious' that it's doing something wrong.

I can see how this feature might appear to be an easy win, but I think there are some downsides that need to be evaluated and it isn't immediately clear to me whether the benefits are worth it.

[posva]

What about making the old value readonly in dev? That way it will avoid the usage of the 3 playgrounds both at runtime and build (with typescript)

[jods4]

But it's not arbitrary: in the case of objects, modifying the old value and returning it doesn't trigger reactivity.

Yes, and it doesn't mean there's never a point in mutating that instance.
I gave the example where that instance is actually a reactive object, that would make sense.
Turning the previous value readonly would actively prevent such use cases.

If the old value passed to the computed is a special readonly we can warn the user about such pitfall and redirect them to the docs

If it's a special readonly (or even a regular readonly, not just TS typing) then you're adding proxy problems on top of that. Please don't introduce non-intuitive proxifying to computed.

Vue doesn't warns and points to docs when a user mutates and re-assigns the same instance to a ref, should it?

[posva]

Yes, and it doesn't mean there's never a point in mutating that instance.

Given the highlighted problems (no reactivity), I think there is not. It's a new concept anyway, so it's fine to introduce rules for its usage.

I gave the example where that instance is actually a reactive object, that would make sense.

In that case things work but I don't think it makes sense to rely on such behavior because of the inconsistencies.

hen you're adding proxy problems on top of that

Which ones? 😅 Even a TS only would be better IMO

[jods4]

It's a new concept anyway,

Can you clarify why you see it as new? I previously expressed the opposite opinion.

Refs work exactly as described in this discussion (same instance -> no trigger); and in my experience I've witnessed several times devs trying to trigger reactivity by re-assigning the same value.

This discussion is in part about educating devs and having a consistent model that is simple to explain helps a lot.
Seeing this as a new concept rather than a natural extension of what already exists is IMHO not the best approach.

In that case things work but I don't think it makes sense to rely on such behavior because of the inconsistencies.

Which inconsistencies? Everything is perfectly consistent:

• Refs and Computed only trigger when their value changes ("change" as understood in normal JS code);
• Mutations to reactive objects are observed.

If you understand this (relatively simple) reactivity model, I don't see why it wouldn't make sense to rely on such behavior?

[Proxy problems] Which ones?

Referential identity is a huge one.
The big gotcha here is someone doing if (previousValue === data), which is pretty natural. Now you need to document and explain to developers why previousValue is actually not exactly the value that was previously returned. This is bad.

TS-only would be the only acceptable approach in my opinion, the drawback of course is that it leaves JS devs out.

[posva]

Can you clarify why you see it as new? I previously expressed the opposite opinion.

I mean the argument being passed to computed.

Which inconsistencies? Everything is perfectly consistent:

I meant the playgrounds: they don't behave as expected. I understand you connect this to how the reactivity works in Vue about just modifying a reactive object just works but the whole idea I'm trying to express is around the fact that the oldValue is not just another reactive value

Referential identity is a huge one.

True, that could break existing code. Then a TS type would be enough. It does leave JS devs out but I find that acceptable as it's not mandatory to have, it's more of a hint. It could also be an eslint rule to include JS devs.

[AlexandreBonaventure]

My 2c on this matter:
I think we can all agree on the fact that mutation side-effects in computed is generally considered a bad practice, but currently nothing really prevents you to write something like that:

const bad = computed(() => {
    somethingElse.value = xxx
    return something.value
})

As such, I would expect user to be already used to not trigger mutation in computed.
I feel like most advanced vue users are already familiar with that to jods4 point. I wouldn't mind to add extra warning, but I'm not sharing the concerns expressed above. some linting rules can also address most of these issues, I think

[yyx990803]

As a general rule of thumb, you should never be mutating anything in a computed getter - we've emphasized that computed getters should be side-effect-free, maybe we just need to be more explicit that side effects include mutating other state.

[skirtles-code]

I think the potential problem here is that a lot of people don't have a clear understanding of what exactly counts as a 'side effect' and what doesn't. It's possible to have a somewhat hand-wavy grasp of what that means and still use computed correctly.

For example, not all mutations are forbidden. Something like this is fine, even though it mutates an object:

const obj = computed(() => {
  const o = {}

  if (foo.value) {
    // Mutation!
    o.bar = true
  }

  return o
})

I suspect many people would use the following line of (slightly flawed) reasoning to justify the correctness of that code:

Modifying the object is OK, because it is created by the computed and belongs to the computed. It only counts as a 'side effect' if you're modifying external state that doesn't belong to the computed.

That's almost right, but not quite. In practice, this reasoning is good enough when making most decisions about what's allowed and what isn't with the pre-3.4 implementation (without the old value).

The old value that we get passed back is also an object created by the same computed. In some sense, it belongs to the computed and it seems natural that the computed is 'managing' that value. I don't think it is immediately obvious why it's no longer OK to mutate that object. It was fine to mutate it when we first created it, so why not now?

There's a transition that happens when the object is first returned. It suddenly becomes 'external'. Any changes after that point are considered side effects, even if those changes are being made by the same computed that created the object. This may seem obvious to more experienced developers, but I think it requires a really good grasp of what counts as a side effect, above and beyond the level of understanding that was required previously. It's not that people don't know to avoid side effects, it's just not always easy to know what counts as a side effect.

The guidance of having 'no side effects' for a computed is useful, but (understandably) that guidance is often stated without a detailed explanation of the underlying motivation behind it. In my mind, one reason for that rule is that Vue places no guarantees on exactly when a computed callback will be called, just that it will always be called in time for when the value is needed. Changes to the reactivity system (e.g. 3.4) can lead to the callback being called at slightly different times, but the callback should be written in such a way that it isn't sensitive to that timing. The same inputs should lead to the same output, no matter how many times it is called, and any superfluous calls should be harmless because it isn't doing anything else (i.e. side effects) apart from computing the return value.

It is possible to implement the 'old value' functionality in userland. Doing that requires some kind of cache. Populating that cache could be regarded as a side effect. If we follow the 'no side effects' rule strictly then we can't implement that in userland code, but I think most people would regard that as an example of an acceptable exception to the rule. If we consider the underlying principles behind that rule (such as those I discussed in the previous paragraph) then it seems that none of those are violated by using a cache. I believe such caches are also commonly used when a computed returns an async component, to ensure the same async wrapper is reused where possible.

The example below shows the flip side of that. It uses the old value to make the computed stateful:

https://play.vuejs.org/#eNp9UstOwzAQ/JXFl7ZSSYV6q9JKgCoBB0CA4IB7qJJt4uLYkR+hKMq/s3aUlgNqTvHM7O7M2i27ruuk8cgWLLWZEbUDi87XK65EVWvjoIVMV7V3mE/B4A462BldwYiKRlxxlWllHVS2gGXgx6M7lFLDhzYyvxhNTpJSWKfND8mGhuOxlvn7Vnok8HMzgeUKWq6ABLEgoEmSDKJN6AVQJl7ZUuzcmIYmTWDiFKDxzhsFJVcdIemsD0RR6OCwquXWIZ0A0vJq1bbRdNelMzpFVCiyBc1lpXOUS86I56ynwkKAPqoacnRdZGaBSmfH/mzKnKUAO1Eke6sVbTZm4izEFhLNU+0EBeRs0acN3JZ29v0QMWc8Tgc8KzH7+gff20PAOHs2aNE0yNmRc1tToOvp9esjHuj/SFI2L0l9hnxBq6UPHnvZjVc52f6ji27v4/sQqniz64NDZYdQwWhQdlHPGb2U2zPRT3bnyTzW0f2x7hcZz93E

I think most people would regard this code as not having any side effects, as the old value is not being mutated. But it does violate the underlying principle that the callback shouldn't be sensitive to superfluous calls. In many ways this example seems acceptable, but it makes some assumptions about the reactivity system that aren't documented and could easily break in a future version of Vue.

I understand that the comments on this discussion have largely focused on the specific example I gave in my initial post, but my concern was slightly broader than that one specific case. More generally, the problem I have with this feature is that the legitimate use cases seem very narrow and there are lots of ways it can be used incorrectly. While I'm sure the docs can be improved to make it clear what is and isn't allowed, I still worry that this feature seems to come with a lot of 'don't do that' caveats, and that's usually a red flag about a feature's design.