fix: change teardown to use onScopeDispose (#1545)

Author: nickmessing

packages/test-e2e-composable-vue3/src/components/ChannelListPiniaContainer.vue

@@ -0,0 +1,25 @@
+<script lang="ts" setup>
+import { ref } from 'vue'
+
+import ChannelListPinia from './ChannelListPinia.vue'
+
+const isComponentVisible = ref(true)
+</script>
+
+<template>
+  <div>
+    <button
+      data-test-id="channel-list-toggle"
+      @click="isComponentVisible = !isComponentVisible"
+    >
+      Toggle ChannelListPinia component
+    </button>
+
+    <div
+      v-if="isComponentVisible"
+      data-test-id="channel-list-container"
+    >
+      <ChannelListPinia />
+    </div>
+  </div>
+</template>

packages/test-e2e-composable-vue3/src/router.ts

@@ -72,6 +72,13 @@ export const router = createRouter({
         layout: 'blank',
       },
     },
+    {
+      path: '/pinia3',
+      component: () => import('./components/ChannelListPiniaContainer.vue'),
+      meta: {
+        layout: 'blank',
+      },
+    },
     {
       path: '/no-setup-scope-query',
       component: () => import('./components/NoSetupScopeQuery.vue'),

packages/test-e2e-composable-vue3/tests/e2e/specs/pinia.cy.ts

@@ -16,4 +16,15 @@ describe('Pinia', () => {
     cy.contains('.channel-link', '# General')
     cy.contains('.channel-link', '# Random')
   })
+
+  it('works after component unmount if used in pinia', () => {
+    cy.visit('/pinia3')
+    cy.get('[data-test-id="channel-list-container"]').should('exist')
+    cy.get('[data-test-id="channel-list-toggle"]').first().click()
+    cy.get('[data-test-id="channel-list-container"]').should('not.exist')
+    cy.get('[data-test-id="channel-list-toggle"]').first().click()
+    cy.get('.channel-link').should('have.lengthOf', 2)
+    cy.contains('.channel-link', '# General')
+    cy.contains('.channel-link', '# Random')
+  })
 })

packages/vue-apollo-composable/src/useMutation.ts

@@ -1,6 +1,6 @@
 import { DocumentNode } from 'graphql'
 import { MutationOptions, OperationVariables, FetchResult, TypedDocumentNode, ApolloError, ApolloClient } from '@apollo/client/core/index.js'
-import { ref, onBeforeUnmount, isRef, Ref, getCurrentInstance, shallowRef } from 'vue-demi'
+import { ref, onScopeDispose, isRef, Ref, getCurrentScope, shallowRef } from 'vue-demi'
 import { useApolloClient } from './useApolloClient'
 import { ReactiveFunction } from './util/ReactiveFunction'
 import { useEventHook } from './util/useEventHook'
@@ -53,9 +53,9 @@ export function useMutation<
   document: DocumentParameter<TResult, TVariables>,
   options: OptionsParameter<TResult, TVariables> = {},
 ): UseMutationReturn<TResult, TVariables> {
-  const vm = getCurrentInstance()
+  const currentScope = getCurrentScope()
   const loading = ref<boolean>(false)
-  vm && trackMutation(loading)
+  currentScope && trackMutation(loading)
   const error = shallowRef<ApolloError | null>(null)
   const called = ref<boolean>(false)
 
@@ -118,7 +118,7 @@ export function useMutation<
     return null
   }
 
-  vm && onBeforeUnmount(() => {
+  currentScope && onScopeDispose(() => {
     loading.value = false
   })
 

packages/vue-apollo-composable/src/useQuery.ts

@@ -5,8 +5,8 @@ import {
   computed,
   watch,
   onServerPrefetch,
-  getCurrentInstance,
-  onBeforeUnmount,
+  getCurrentScope,
+  onScopeDispose,
   nextTick,
   shallowRef,
 } from 'vue-demi'
@@ -34,8 +34,6 @@ import { trackQuery } from './util/loadingTracking'
 import { resultErrorsToApolloError, toApolloError } from './util/toApolloError'
 import { isServer } from './util/env'
 
-import type { CurrentInstance } from './util/types'
-
 export interface UseQueryOptions<
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   TResult = any,
@@ -152,8 +150,7 @@ export function useQueryImpl<
   options: OptionsParameter<TResult, TVariables> = {},
   lazy = false,
 ): UseQueryReturn<TResult, TVariables> {
-  // Is on server?
-  const vm = getCurrentInstance() as CurrentInstance | null
+  const currentScope = getCurrentScope()
 
   const currentOptions = ref<UseQueryOptions<TResult, TVariables>>()
 
@@ -176,7 +173,7 @@ export function useQueryImpl<
    * Indicates if a network request is pending
    */
   const loading = ref(false)
-  vm && trackQuery(loading)
+  currentScope && trackQuery(loading)
   const networkStatus = ref<number>()
 
   // SSR
@@ -202,7 +199,7 @@ export function useQueryImpl<
     firstRejectError = undefined
   }
 
-  vm && onServerPrefetch?.(() => {
+  currentScope && onServerPrefetch?.(() => {
     if (!isEnabled.value || (isServer && currentOptions.value?.prefetch === false)) return
 
     return new Promise<void>((resolve, reject) => {
@@ -615,10 +612,14 @@ export function useQueryImpl<
   }
 
   // Teardown
-  vm && onBeforeUnmount(() => {
-    stop()
-    subscribeToMoreItems.length = 0
-  })
+  if (currentScope) {
+    onScopeDispose(() => {
+      stop()
+      subscribeToMoreItems.length = 0
+    })
+  } else {
+    console.warn('[Vue apollo] useQuery() is called outside of an active effect scope and the query will not be automatically stopped.')
+  }
 
   return {
     result,

packages/vue-apollo-composable/src/useSubscription.ts

@@ -5,8 +5,8 @@ import {
   watch,
   isRef,
   computed,
-  getCurrentInstance,
-  onBeforeUnmount,
+  getCurrentScope,
+  onScopeDispose,
   nextTick,
   shallowRef,
 } from 'vue-demi'
@@ -27,7 +27,6 @@ import { paramToReactive } from './util/paramToReactive'
 import { useApolloClient } from './useApolloClient'
 import { useEventHook } from './util/useEventHook'
 import { trackSubscription } from './util/loadingTracking'
-import type { CurrentInstance } from './util/types'
 import { toApolloError } from './util/toApolloError'
 import { isServer } from './util/env'
 
@@ -121,8 +120,7 @@ export function useSubscription <
   variables: VariablesParameter<TVariables> | undefined = undefined,
   options: OptionsParameter<TResult, TVariables> = {},
 ): UseSubscriptionReturn<TResult, TVariables> {
-  // Is on server?
-  const vm = getCurrentInstance() as CurrentInstance | null
+  const currentScope = getCurrentScope()
 
   const documentRef = paramToRef(document)
   const variablesRef = paramToRef(variables)
@@ -134,7 +132,7 @@ export function useSubscription <
   const errorEvent = useEventHook<[ApolloError, OnErrorContext]>()
 
   const loading = ref(false)
-  vm && trackSubscription(loading)
+  currentScope && trackSubscription(loading)
 
   // Apollo Client
   const { resolveClient } = useApolloClient()
@@ -298,7 +296,11 @@ export function useSubscription <
   })
 
   // Teardown
-  vm && onBeforeUnmount(stop)
+  if (currentScope) {
+    onScopeDispose(stop)
+  } else {
+    console.warn('[Vue apollo] useSubscription() is called outside of an active effect scope and the subscription will not be automatically stopped.')
+  }
 
   return {
     result,

packages/vue-apollo-composable/src/util/loadingTracking.ts

@@ -1,14 +1,16 @@
-import { Ref, watch, onUnmounted, ref, getCurrentInstance, onBeforeUnmount } from 'vue-demi'
+import { Ref, watch, onUnmounted, ref, getCurrentScope, onScopeDispose } from 'vue-demi'
 import { isServer } from './env.js'
 
+import type { EffectScope } from 'vue-demi'
+
 export interface LoadingTracking {
   queries: Ref<number>
   mutations: Ref<number>
   subscriptions: Ref<number>
 }
 
 export interface AppLoadingTracking extends LoadingTracking {
-  components: Map<any, LoadingTracking>
+  components: Map<EffectScope, LoadingTracking>
 }
 
 export const globalTracking: AppLoadingTracking = {
@@ -19,26 +21,26 @@ export const globalTracking: AppLoadingTracking = {
 }
 
 export function getCurrentTracking () {
-  const vm = getCurrentInstance()
-  if (!vm) {
+  const currentScope = getCurrentScope()
+  if (!currentScope) {
     return {}
   }
 
   let tracking: LoadingTracking
 
-  if (!globalTracking.components.has(vm)) {
+  if (!globalTracking.components.has(currentScope)) {
     // Add per-component tracking
-    globalTracking.components.set(vm, tracking = {
+    globalTracking.components.set(currentScope, tracking = {
       queries: ref(0),
       mutations: ref(0),
       subscriptions: ref(0),
     })
     // Cleanup
     onUnmounted(() => {
-      globalTracking.components.delete(vm)
+      globalTracking.components.delete(currentScope)
     })
   } else {
-    tracking = globalTracking.components.get(vm) as LoadingTracking
+    tracking = globalTracking.components.get(currentScope) as LoadingTracking
   }
 
   return {
@@ -61,7 +63,7 @@ function track (loading: Ref<boolean>, type: keyof LoadingTracking) {
     immediate: true,
   })
 
-  onBeforeUnmount(() => {
+  onScopeDispose(() => {
     if (loading.value) {
       if (tracking) tracking[type].value--
       globalTracking[type].value--