refactor: make effect scope enable/disable logic symmetric

Author: johnsoncodehk

packages/reactivity/__tests__/effectScope.spec.ts

@@ -9,6 +9,7 @@ import {
   onScopeDispose,
   reactive,
   ref,
+  setCurrentScope,
 } from '../src'
 
 describe('reactivity/effect/scope', () => {
@@ -289,21 +290,7 @@ describe('reactivity/effect/scope', () => {
 
     parentScope.run(() => {
       const childScope = effectScope(true)
-      childScope.on()
-      childScope.off()
-      expect(getCurrentScope()).toBe(parentScope)
-    })
-  })
-
-  it('calling on() and off() multiple times inside an active scope should not break currentScope', () => {
-    const parentScope = effectScope()
-    parentScope.run(() => {
-      const childScope = effectScope(true)
-      childScope.on()
-      childScope.on()
-      childScope.off()
-      childScope.off()
-      childScope.off()
+      setCurrentScope(setCurrentScope(childScope))
       expect(getCurrentScope()).toBe(parentScope)
     })
   })

packages/reactivity/src/effectScope.ts

@@ -20,10 +20,6 @@ export class EffectScope implements Subscriber, Dependency {
   subs: Link | undefined = undefined
   subsTail: Link | undefined = undefined
 
-  /**
-   * @internal track `on` calls, allow `on` call multiple times
-   */
-  private _on = 0
   /**
    * @internal
    */
@@ -87,29 +83,6 @@ export class EffectScope implements Subscriber, Dependency {
     }
   }
 
-  prevScope: EffectScope | undefined
-  /**
-   * This should only be called on non-detached scopes
-   * @internal
-   */
-  on(): void {
-    if (++this._on === 1) {
-      this.prevScope = activeEffectScope
-      activeEffectScope = this
-    }
-  }
-
-  /**
-   * This should only be called on non-detached scopes
-   * @internal
-   */
-  off(): void {
-    if (this._on > 0 && --this._on === 0) {
-      activeEffectScope = this.prevScope
-      this.prevScope = undefined
-    }
-  }
-
   stop(): void {
     if (this.active) {
       this.flags |= EffectFlags.STOP
@@ -150,6 +123,14 @@ export function getCurrentScope(): EffectScope | undefined {
   return activeEffectScope
 }
 
+export function setCurrentScope(
+  scope: EffectScope | undefined,
+): EffectScope | undefined {
+  const prevScope = activeEffectScope
+  activeEffectScope = scope
+  return prevScope
+}
+
 /**
  * Registers a dispose callback on the current active effect scope. The
  * callback will be invoked when the associated effect scope is stopped.

packages/reactivity/src/index.ts

@@ -76,6 +76,10 @@ export {
   effectScope,
   EffectScope,
   getCurrentScope,
+  /**
+   * @internal
+   */
+  setCurrentScope,
   onScopeDispose,
 } from './effectScope'
 export { reactiveReadArray, shallowReadArray } from './arrayInstrumentations'

packages/runtime-core/src/apiLifecycle.ts

@@ -37,11 +37,11 @@ export function injectHook(
         // Set currentInstance during hook invocation.
         // This assumes the hook does not synchronously trigger other hooks, which
         // can only be false when the user does something really funky.
-        const reset = setCurrentInstance(target)
+        const prev = setCurrentInstance(target)
         try {
           return callWithAsyncErrorHandling(hook, target, type, args)
         } finally {
-          reset()
+          setCurrentInstance(...prev)
           resetTracking()
         }
       })

packages/runtime-core/src/apiSetupHelpers.ts

@@ -14,7 +14,6 @@ import {
   createSetupContext,
   getCurrentGenericInstance,
   setCurrentInstance,
-  unsetCurrentInstance,
 } from './component'
 import type { EmitFn, EmitsOptions, ObjectEmitsOptions } from './componentEmits'
 import type {
@@ -511,7 +510,7 @@ export function withAsyncContext(getAwaitable: () => any): [any, () => void] {
     )
   }
   let awaitable = getAwaitable()
-  unsetCurrentInstance()
+  setCurrentInstance(null, undefined)
   if (isPromise(awaitable)) {
     awaitable = awaitable.catch(e => {
       setCurrentInstance(ctx)

packages/runtime-core/src/apiWatch.ts

@@ -253,9 +253,9 @@ export function instanceWatch(
     cb = value.handler as Function
     options = value
   }
-  const reset = setCurrentInstance(this)
+  const prev = setCurrentInstance(this)
   const res = doWatch(getter, cb.bind(publicThis), options)
-  reset()
+  setCurrentInstance(...prev)
   return res
 }
 

packages/runtime-core/src/component.ts

@@ -97,7 +97,6 @@ import type { RendererElement } from './renderer'
 import {
   setCurrentInstance,
   setInSSRSetupState,
-  unsetCurrentInstance,
 } from './componentCurrentInstance'
 
 export * from './componentCurrentInstance'
@@ -891,7 +890,7 @@ function setupStatefulComponent(
     pauseTracking()
     const setupContext = (instance.setupContext =
       setup.length > 1 ? createSetupContext(instance) : null)
-    const reset = setCurrentInstance(instance)
+    const prev = setCurrentInstance(instance)
     const setupResult = callWithErrorHandling(
       setup,
       instance,
@@ -903,14 +902,17 @@ function setupStatefulComponent(
     )
     const isAsyncSetup = isPromise(setupResult)
     resetTracking()
-    reset()
+    setCurrentInstance(...prev)
 
     if ((isAsyncSetup || instance.sp) && !isAsyncWrapper(instance)) {
       // async setup / serverPrefetch, mark as async boundary for useId()
       markAsyncBoundary(instance)
     }
 
     if (isAsyncSetup) {
+      const unsetCurrentInstance = (): void => {
+        setCurrentInstance(null, undefined)
+      }
       setupResult.then(unsetCurrentInstance, unsetCurrentInstance)
       if (isSSR) {
         // return the promise so server-renderer can wait on it
@@ -1083,13 +1085,13 @@ export function finishComponentSetup(
 
   // support for 2.x options
   if (__FEATURE_OPTIONS_API__ && !(__COMPAT__ && skipOptions)) {
-    const reset = setCurrentInstance(instance)
+    const prev = setCurrentInstance(instance)
     pauseTracking()
     try {
       applyOptions(instance)
     } finally {
       resetTracking()
-      reset()
+      setCurrentInstance(...prev)
     }
   }
 

packages/runtime-core/src/componentCurrentInstance.ts

@@ -4,6 +4,7 @@ import type {
   GenericComponentInstance,
 } from './component'
 import { currentRenderingInstance } from './componentRenderContext'
+import { type EffectScope, setCurrentScope } from '@vue/reactivity'
 
 /**
  * @internal
@@ -25,7 +26,10 @@ export let isInSSRComponentSetup = false
 
 export let setInSSRSetupState: (state: boolean) => void
 
-let internalSetCurrentInstance: (
+/**
+ * @internal
+ */
+export let simpleSetCurrentInstance: (
   instance: GenericComponentInstance | null,
 ) => void
 
@@ -53,7 +57,7 @@ if (__SSR__) {
       else setters[0](v)
     }
   }
-  internalSetCurrentInstance = registerGlobalSetter(
+  simpleSetCurrentInstance = registerGlobalSetter(
     `__VUE_INSTANCE_SETTERS__`,
     v => (currentInstance = v),
   )
@@ -66,42 +70,22 @@ if (__SSR__) {
     v => (isInSSRComponentSetup = v),
   )
 } else {
-  internalSetCurrentInstance = i => {
+  simpleSetCurrentInstance = i => {
     currentInstance = i
   }
   setInSSRSetupState = v => {
     isInSSRComponentSetup = v
   }
 }
 
-export const setCurrentInstance = (instance: GenericComponentInstance) => {
+export const setCurrentInstance = (
+  instance: GenericComponentInstance | null,
+  scope: EffectScope | undefined = instance !== null
+    ? instance.scope
+    : undefined,
+): [GenericComponentInstance | null, EffectScope | undefined] => {
   const prev = currentInstance
-  internalSetCurrentInstance(instance)
-  instance.scope.on()
-  return (): void => {
-    instance.scope.off()
-    internalSetCurrentInstance(prev)
-  }
-}
-
-export const unsetCurrentInstance = (): void => {
-  currentInstance && currentInstance.scope.off()
-  internalSetCurrentInstance(null)
-}
-
-/**
- * Exposed for vapor only. Vapor never runs during SSR so we don't want to pay
- * for the extra overhead
- * @internal
- */
-export const simpleSetCurrentInstance = (
-  i: GenericComponentInstance | null,
-  unset?: GenericComponentInstance | null,
-): void => {
-  currentInstance = i
-  if (unset) {
-    unset.scope.off()
-  } else if (i) {
-    i.scope.on()
-  }
+  simpleSetCurrentInstance(instance)
+  const prevScope = setCurrentScope(scope)
+  return [prev, prevScope]
 }

packages/runtime-core/src/componentProps.ts

@@ -522,15 +522,15 @@ function baseResolveDefault(
   key: string,
 ) {
   let value
-  const reset = setCurrentInstance(instance)
+  const prev = setCurrentInstance(instance)
   const props = toRaw(instance.props)
   value = factory.call(
     __COMPAT__ && isCompatEnabled(DeprecationTypes.PROPS_DEFAULT_THIS, instance)
       ? createPropsDefaultThis(instance, props, key)
       : null,
     props,
   )
-  reset()
+  setCurrentInstance(...prev)
   return value
 }
 

packages/runtime-core/src/index.ts

@@ -543,6 +543,7 @@ export {
  */
 export {
   currentInstance,
+  setCurrentInstance,
   simpleSetCurrentInstance,
 } from './componentCurrentInstance'
 /**

packages/runtime-core/src/renderer.ts

@@ -59,6 +59,7 @@ import {
   ReactiveEffect,
   pauseTracking,
   resetTracking,
+  setCurrentScope,
 } from '@vue/reactivity'
 import { updateProps } from './componentProps'
 import { updateSlots } from './componentSlots'
@@ -1594,9 +1595,9 @@ function baseCreateRenderer(
     }
 
     // create reactive effect for rendering
-    instance.scope.on()
+    const prevScope = setCurrentScope(instance.scope)
     const effect = (instance.effect = new ReactiveEffect(componentUpdateFn))
-    instance.scope.off()
+    setCurrentScope(prevScope)
 
     const update = (instance.update = effect.run.bind(effect))
     const job: SchedulerJob = (instance.job = effect.scheduler.bind(effect))

packages/runtime-vapor/__tests__/dom/prop.spec.ts

@@ -12,20 +12,13 @@ import {
 } from '../../src/dom/prop'
 import { setStyle } from '../../src/dom/prop'
 import { VaporComponentInstance } from '../../src/component'
-import {
-  currentInstance,
-  ref,
-  simpleSetCurrentInstance,
-} from '@vue/runtime-dom'
+import { ref, setCurrentInstance } from '@vue/runtime-dom'
 
 let removeComponentInstance = NOOP
 beforeEach(() => {
   const instance = new VaporComponentInstance({}, {}, null)
-  const prev = currentInstance
-  simpleSetCurrentInstance(instance)
-  removeComponentInstance = () => {
-    simpleSetCurrentInstance(prev)
-  }
+  const prev = setCurrentInstance(instance)
+  removeComponentInstance = () => setCurrentInstance(...prev)
 })
 afterEach(() => {
   removeComponentInstance()

packages/runtime-vapor/src/component.ts

@@ -20,7 +20,7 @@ import {
   pushWarningContext,
   queuePostFlushCb,
   registerHMR,
-  simpleSetCurrentInstance,
+  setCurrentInstance,
   startMeasure,
   unregisterHMR,
   warn,
@@ -191,8 +191,7 @@ export function createComponent(
     instance.emitsOptions = normalizeEmitsOptions(component)
   }
 
-  const prev = currentInstance
-  simpleSetCurrentInstance(instance)
+  const prev = setCurrentInstance(instance)
   pauseTracking()
 
   if (__DEV__) {
@@ -260,7 +259,7 @@ export function createComponent(
   }
 
   resetTracking()
-  simpleSetCurrentInstance(prev, instance)
+  setCurrentInstance(...prev)
 
   if (__DEV__) {
     popWarningContext()

packages/runtime-vapor/src/componentProps.ts

@@ -12,12 +12,11 @@ import type { VaporComponent, VaporComponentInstance } from './component'
 import {
   type NormalizedPropsOptions,
   baseNormalizePropsOptions,
-  currentInstance,
   isEmitListener,
   popWarningContext,
   pushWarningContext,
   resolvePropValue,
-  simpleSetCurrentInstance,
+  setCurrentInstance,
   validateProps,
   warn,
 } from '@vue/runtime-dom'
@@ -257,10 +256,9 @@ function resolveDefault(
   factory: (props: Record<string, any>) => unknown,
   instance: VaporComponentInstance,
 ) {
-  const prev = currentInstance
-  simpleSetCurrentInstance(instance)
+  const prev = setCurrentInstance(instance)
   const res = factory.call(null, instance.props)
-  simpleSetCurrentInstance(prev, instance)
+  setCurrentInstance(...prev)
   return res
 }