[Bug report] computedWithControl is triggering "Computed is still dirty after getter evaluation" warning

[meteorlxy]

Describe the bug

Related issue: vuejs/core#10341

computedWithControl will trigger the warning in vue 3.4.19:

[Vue warn] Computed is still dirty after getter evaluation, likely because a computed is mutating its own dependency in its getter. State mutations in computed getters should be avoided.  Check the docs for more details: https://vuejs.org/guide/essentials/computed.html#getters-should-be-side-effect-free

Reproduction

https://stackblitz.com/edit/vitejs-vite-bvw5gp?file=src%2FApp.vue

System Info

N/A

Used Package Manager

npm

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a VueUse issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to https://github.com/vuejs/core instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion.
• The provided reproduction is a minimal reproducible example of the bug.

[meteorlxy]

cc @Doctor-wu

[Doctor-wu]

@meteorlxy Hey, computedWithControl is not a computed under the hood, it just combine the usage of watch and computed. computedWithControl has side-effect since it need to maintain some refs internally. So it's not suggested to use computedWithControl in a real computed, you can use watch which is designed for side-effect operation, or use syncRef for ease.

[Doctor-wu]

Here is an example

[Doctor-wu]

Submitted a PR for vuepress case
vuepress/core#1501

[meteorlxy]

@Doctor-wu I see. Thanks for looking into that.

It looks like a workaround in userland code. Is it possible to fix in vueuse side? If not, we'd better put a warning in the docs.

[Doctor-wu]

@Doctor-wu I see. Thanks for looking into that.

It looks like a workaround in userland code. Is it possible to fix in vueuse side? If not, we'd better put a warning in the docs.

A warning is a good point. But in fact, lots of function in vueuse have side effects, so it relies on user's judgment more often.

[Mister-Hope]

I suggest not closing this issue too casually. it's weird to say:

computedWithControl is not a computed under the hood.

Then why a name with computedWithControl? There are people reporting the same problem with VueUse functions (see #3774 (comment)). I don't think add a warning somewhere in computedWithControl docs helps.

I would suggest one of the 2 things which should really solve the issue:

• try to make computedWithControl work with Vue 3.4 new reactivity system
• deprecate computedWithControl and any other api with /** @deprecated */ which may confuse the users, and give them new name to better describe them and maybe warnings in jsdocs too.

As long as computedWithControl and things like this is returned in any VueUse functions, then it will probably be misused in user cases. You can not expect users to special handle them.

[Doctor-wu]

@Mister-Hope

Then why a name with computedWithControl?

It just combines the usage of watch and computed, to help users understand how to use this function. It uses customRef to implement a controllable "computed" usage-like ref.

This function was added to vueuse almost 2 years ago, and at that time, vue didn't have a new reactivity system yet, so it's hard to say it's contributors misuse.

try to make computedWithControl work with Vue 3.4 new reactivity system

computedWithControl works well in the new reactivity system, it's reasonable to have side-effects to implement a function like this. The warning was caused by user's misuse in computed, so it make no sense to mark this function as deprecated as well.

[Mister-Hope]

Again to clarify:

1. computed can be used in another computed
2. computedWithControl sounds like a computed
3. Even if users do not manually use computedWithControl (in that case a docs warning would be fine), they can still get it from VueUse functions
4. Using computedWithControl inside computed is wrong, people can only watch or watchEffect with it.

People will probably misuse it under 1, 2, 3, so it leads to 4

That's the problem I am talking, again, unless VueUse only use this one internally, otherwise I don't think I am convinced by any of the reasons above.

[Doctor-wu]

I think mark this function is not a computed will be helpful, thanks for your suggestions!

[Mister-Hope]

Yes, that's something I want to express, I do think that VueUse should do something to avoid people misuse them, for example, the useActiveElement (here: https://vueuse.org/core/useactiveelement/#useactiveelement) use watch in examples, but you can not expect developers to "understand" that we can only watch this and should never generated other state using computed with it.

It should be a historical problems, so hope you can open another issue to track it and help the community, while I am facing the problem in VuePress developing, I also notice a lot of similar noise upstream in Vue core about it after vue@3.4.19 raise a warning about it. Some of the issues are misuse of computed by users, while some of them are related to VueUse. I hope the second one can be nicely warned here.

[Mister-Hope]

At least in my side, I believe returning a computedWithControl could be a bad thing for users, just take the useActiveElement as example, we can definitely return a standard computed so that users can generate other state with computed.😉 Can this be considered, and do you think a full check in all apis is helpful?

[Doctor-wu]

That's a hard one to check the full apis only by team, it's more feasible to rely on the power of community, PR are welcome for solve some historical problems like this.

while some of them are related to VueUse

For this part, if you find vueuse misused new reactivity instead of misused by userland's code, it's greatly encouraged to submit issues.

What ever, I will check useActiveElement case later.

[p1xel007]

I'm getting this warning while using "useStorage" form VueUse @Doctor-wu

[Doctor-wu]

I'm getting this warning while using "useStorage" form VueUse @Doctor-wu

Did you use useStorage in computed, or it just throw this warning by itself?

[p1xel007]

I'm getting this warning while using "useStorage" form VueUse @Doctor-wu

Did you use useStorage in computed, or it just throw this warning by itself?

I'm using it in Pinia store inside getter

[Doctor-wu]

I'm getting this warning while using "useStorage" form VueUse @Doctor-wu

Did you use useStorage in computed, or it just throw this warning by itself?

I'm using it in Pinia store inside getter

Can you show me your code about this?
As a side note, Pinia's useStore has side effect itself, look at this for more information vuejs/core#10341

[p1xel007]

I'm getting this warning while using "useStorage" form VueUse @Doctor-wu

Did you use useStorage in computed, or it just throw this warning by itself?

I'm using it in Pinia store inside getter

Can you show me your code about this? As a side note, Pinia's useStore has side effect itself, look at this for more information vuejs/core#10341

Pinia store:

`import { defineStore } from 'pinia';
import { useStorage } from '@vueuse/core';

export const UserStore = defineStore({
id: 'user',

state: () => ({
    user_data: {}
}),

getters: {
    isLoggedIn: () => useStorage('isLoggedIn')
},

actions: {}

});
export default UserStore;
`

This is how I called it: (And this is where the warning shows from)

const userLoggedIn = UserStore().isLoggedIn;

[Doctor-wu]

Is there any warning if you just returned a static value like

  getters: {
    isLoggedIn: () => true,
  }

[p1xel007]

Is there any warning if you just returned a static value like

  getters: {
    isLoggedIn: () => true,
  }

No, the warning disappear when I return anything other than VueUse function

like as a warkaround I'm using localStorage.getItem('isLoggedIn') instead

[Doctor-wu]

In fact, getters in Pinia is a computed as well, so it will cause that warning.

[p1xel007]

In fact, getters in Pinia is a computed as well, so it will cause that warning.

I see, what's the approach to resolve this warning?

[Doctor-wu]

You can extract the useStorage outside the getters like on the top of the file, and use the return value in the getters.

  const isLoggedInRef = useStorage('isLoggedIn')
  ...
  getters: {
    isLoggedIn: () => isLoggedInRef.value
  }

[Doctor-wu]

BTW, it's not suggested to directly reply under a closed issue, you can create a new one next time.