Preventing problems accessing the current instance in a computed

[skirtles-code]

There's a problem I've seen a few times on Vue Land with using computed and I'm wondering whether there's anything we can do in Vue core to mitigate it.

The following code invokes a composable inside a computed callback:

const c1 = computed(() => useFoo())

There's a potential problem with this code. A composable (typically) needs to be invoked during setup, rendering or a lifecycle hook. A computed can't currently guarantee this, it would depend on when exactly the value is accessed.

Here's a Playground example that shows the potential problem:

• SFC Playground

Both c1 and c2 work fine, but c3 does not. It all comes down to whether getCurrentInstance() returns the relevant component at the moment the value is accessed.

In my opinion, a computed shouldn't have a dependency on getCurrentInstance(). But this is an easy trap to fall into as the code appears to work in most cases.

We saw an example like this on Vue Land yesterday where someone was calling useI18n() inside a computed. Their code worked in 3.3 but failed in 3.4. Sadly I don't have a reproduction, but it's easy to imagine that some small difference in the timing of the evaluation of the computed could lead to this failing. From their perspective, this appears to be a regression in Vue.

I think it's worth considering how we might try to help people avoid this pitfall.

Idea 1 - set current instance to null

We could set the current instance to null when evaluating a computed. I suspect this would make a lot of people unhappy when their existing code blows up.

Idea 2 - Have getCurrentInstance() log a warning

We could log a warning when trying to access getCurrentInstance() in a computed.

This might not be as clean as it sounds because sometimes that function is called in cases where it doesn't necessarily matter what it returns (similar to hasInjectionContext()). For example, it might be called just to provide extra debug information during development, and we wouldn't want to log a warning in that case.

Idea 3 - Set the current instance consistently

A computed created inside setup is already implicitly tied to the component via the dispose context. We could extend that link further by always setting the current instance to be the component before evaluating the computed. For a computed created outside a component it could set the current instance to null.

This is similar to idea 1, except that it'll lead to far fewer breakages. The downside is that it doesn't really do anything to discourage the dodgy code being written, it just ensures it'll behave consistently.

If this idea does end up being implemented, perhaps other effects (e.g. watch/watchEffect) should work the same way?

Idea 4 - Log a warning in provide/inject/onMounted

This is similar to idea 2, but more targeted.

Rather than logging a warning for getCurrentInstance() calls, instead log a warning in the specific functions we care about, e.g. provide/inject/onMounted/etc., if they are called during the execution of a computed callback. This is somewhat similar to the existing warnings that are logged when the current instance doesn't exist, but covering the case where it exists coincidentally.

[speedpro]

I like Idea 3. I have a project where some components suddenly stopped working because I upgraded to vue 3.4 and the UI library is relying on having access to getCurrentInstance() within the computed.

If we set the currentInstance for the computed() values, then it would be backwards compatible and also align with the way that computed props in the Options API have access to this.

Out of curiosity, what specific pitfalls do you see with relying on currentInstance within computed? Is it the potential for reactivity to trigger due to changes on the instance?

[Akryum]

getCurrentInstance is sort of private (and it's not documented) and imo belongs to the internals of vue core and should not be relied upon unless absolutely necessary. Composables are supposed to be called in setup or lifecycle hooks as documented here.
Also if you use a function instead of a computed (for lightness purpose) your code will almost certainly break.

[yyx990803]

I went with option (2) in 324e817, as the cases where it's used as a check now has hasInjectionContext as a replacement.

[skirtles-code]

It seems I was mistaken. inject() doesn't use getCurrentInstance(), it just accesses currentInstance || currentRenderingInstance directly. It's not entirely clear to me why that is, but it looks like the inject code predates currentRenderingInstance being added to getCurrentInstance(), so it may just be that.

Perhaps inject() should be changed to go through getCurrentInstance() too, so that it gets the warning?

[pdesmarais]

So, after updating to vue 3.4.6 from 3.3.9, I'm getting loads of warnings
getCurrentInstance() called inside a computed getter. This is incorrect usage as computed getters are not guaranteed to be executed with an active component instance. If you are using a composable inside a computed getter, move it ouside to the setup scope.

I traced it down to using pinia stores outside of vue components. Quick explanation, I have many classes that are dependent on some of our pinia stores. Inside a method in the class which requires the store, there's a call to the useStore for the required store. Everything works fine. I'm not calling getCurrentInstance anywhere but Pinia must be doing this internally since I get the warning on every call to one of my useStore in all of my classes.

Any idea how I should/could fix this?

Thanks!

[yyx990803]

Looks like Pinia calls getCurrentInstance() in useStore for devtools purposes. We may need to revert this while we figure out how to best deal with this in Pinia.

[yyx990803]

The PR also seems to break Vuetify in ecosystem-ci for some reason. Looks like the safer version here is option (4), merging #10038 while reverting the getCurrentInstance warning

[yyx990803]

Oh nvm, Pinia also uses inject() in useStore(), although guarded with hasInjectionContext() it would still cause a warning in #10038

[skirtles-code]

Is calling useStore inside a computed correct usage? Shouldn't that be moved outside the computed just like any other composable?

[yyx990803]

It technically is - this only happens in the mapXXX helpers like mapStores for options API. We will need to find a way to refactor the implementation. /cc @posva

[skirtles-code]

Yeah, makes sense.

I guess the problem for those mappers is that they run in the top-level scope of an ES module, so they're running before the Pinia instance is created. Having the useStore call inside the computed defers it long enough for Pinia to be ready.

Something similar can happen using a global computed without the helpers:

Playground for 10038

That example can be 'fixed' by wrapping double in a composable, but I don't see an easy way to keep code like that working without adding that extra wrapper.

[Ingramz]

Since it is unclear whether OP managed to reproduce the vue-i18n issue, I'm posting a variant I was able to find fairly easily within my projects where 3.4.6 now displays a warning: https://stackblitz.com/edit/vitejs-vite-kdegwj?file=src%2Fmain.ts,src%2FApp.vue

I'm not trying to imply that vue-i18n's internal usage of getCurrentInstance() is valid, but perhaps it will be useful for you as another data point if you wish to validate any of the future changes.

[franck102]

I am definitely seeing that warning with i18n. And I don't see what code I could move out my computed getter to resolve the issue??

Here are the stack trace and the relevant code snippets; I created my own i18n.ts (see below) in an apparently misguided attempt at adressing issues I had trying to use i18n outside of components.

at <PassGrants pass= {id: 729, name: 'Pass local', personal: true, dirty: true, grants: Array(1), …} > 
 at <PassSummary passId=729 onVnodeUnmounted=fn<onVnodeUnmounted> ref=Ref< Proxy(Object) {__v_skip: true} > > 
 at <RouterView key=0 > 
 at <App>
warn$1 @ runtime-core.esm-bundler.js:39
getCurrentInstance @ runtime-core.esm-bundler.js:7537
getMetaInfo @ vue-i18n.mjs:285
wrapWithDeps @ vue-i18n.mjs:489
t @ vue-i18n.mjs:551
isoPeriodToHuman @ date-utils.ts:81
(anonymous) @ PassGrants.vue:101
(anonymous) @ runtime-core.esm-bundler.js:7892
(anonymous) @ reactivity.esm-bundler.js:982
run @ reactivity.esm-bundler.js:170
get value @ reactivity.esm-bundler.js:993
unref @ reactivity.esm-bundler.js:1106
get @ reactivity.esm-bundler.js:1112
_sfc_render @ PassGrants.vue:13
renderComponentRoot @ runtime-core.esm-bundler.js:871
componentUpdateFn @ runtime-core.esm-bundler.js:6101

date-utils.ts:

import { i18n } from "@/common-services/i18n"

export function useDateUtils() {
    const isoPeriodToHuman = (isoDurationStr: string) => {
        ...
        strs.push(i18n.t("XDay", parsed.days))
        ...
    }
    return { ... isoPeriodToHuman ... }
}

PassGrants.vue:

const { isoPeriodToHuman } = useDateUtils();

const rows = computed(() => {
    ....
    validity: isoPeriodToHuman(grant?.validFor)
    ...
});

common-services/i18n.ts:

import { createI18n } from "vue-i18n"
import messages from "@/locales/fr"

const instance = createI18n({
    locale: 'fr',
    legacy: false,
    useScope: "global",
    globalInjection: true,
    messages,
});

export default instance;

export const i18n = instance.global;

[skirtles-code]

@franck102 That warning was added in 3.4.6 and removed in 3.4.7 (see Evan's message below about reverting the change). You should be able to avoid it by upgrading to the latest release, or using any other release apart from 3.4.6.

[yyx990803]

I took a deeper look at the Pinia use cases (and also at the vue-i18n) case by @Ingramz , and I found that both are using getCurrentInstance() in a try...catch manner - i.e. it guards against the possibility of currentInstance being null and only performs optional debugging work, just like @skirtles-code described in option (2). Warning against all such cases seem too harsh indeed.

I've reverted the warning for now in 2fd3905 before we figure out a safer way to warn users against actual wrong usage.

[Xenossolitarius]

Also info if it may be of any help to all of you: we are using Nuxt, I18n, headless-ui and pinia in our project and all I can say is that although SSR somewhat passes with the way we are using the client side just dies on some interactions, to be more precise where we are using uI18n in pinia stores. Also headless-ui is spewing warnings like crazy. Seems to also be impacted

[maicss]

It seems that this not only happens with Pinia. I am not using Pinia in my Nuxt project, still get this warning:

[yyx990803]

The warning has been removed in 3.4.7+.