refactor: make effect scope enable/disable logic symmetric

Author: johnsoncodehk

Diff for: 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)
     })
   })

Diff for: 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.

Diff for: packages/reactivity/src/index.ts

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

Diff for: packages/runtime-core/src/apiSetupHelpers.ts

@@ -14,7 +14,7 @@ import {
   createSetupContext,
   getCurrentGenericInstance,
   setCurrentInstance,
-  unsetCurrentInstance,
+  simpleSetCurrentInstance,
 } from './component'
 import type { EmitFn, EmitsOptions, ObjectEmitsOptions } from './componentEmits'
 import type {
@@ -31,7 +31,7 @@ import type {
 } from './componentProps'
 import { warn } from './warning'
 import type { SlotsType, StrictUnwrapSlotsType } from './componentSlots'
-import type { Ref } from '@vue/reactivity'
+import { type Ref, setCurrentScope } from '@vue/reactivity'
 
 // dev only
 const warnRuntimeUsage = (method: string) =>
@@ -511,7 +511,8 @@ export function withAsyncContext(getAwaitable: () => any): [any, () => void] {
     )
   }
   let awaitable = getAwaitable()
-  unsetCurrentInstance()
+  setCurrentScope(undefined)
+  simpleSetCurrentInstance(null)
   if (isPromise(awaitable)) {
     awaitable = awaitable.catch(e => {
       setCurrentInstance(ctx)

Diff for: packages/runtime-core/src/component.ts

@@ -8,6 +8,7 @@ import {
   pauseTracking,
   proxyRefs,
   resetTracking,
+  setCurrentScope,
   shallowReadonly,
   track,
 } from '@vue/reactivity'
@@ -97,7 +98,7 @@ import type { RendererElement } from './renderer'
 import {
   setCurrentInstance,
   setInSSRSetupState,
-  unsetCurrentInstance,
+  simpleSetCurrentInstance,
 } from './componentCurrentInstance'
 
 export * from './componentCurrentInstance'
@@ -911,6 +912,10 @@ function setupStatefulComponent(
     }
 
     if (isAsyncSetup) {
+      const unsetCurrentInstance = (): void => {
+        setCurrentScope(undefined)
+        simpleSetCurrentInstance(null)
+      }
       setupResult.then(unsetCurrentInstance, unsetCurrentInstance)
       if (isSSR) {
         // return the promise so server-renderer can wait on it

Diff for: packages/runtime-core/src/componentCurrentInstance.ts

@@ -4,6 +4,7 @@ import type {
   GenericComponentInstance,
 } from './component'
 import { currentRenderingInstance } from './componentRenderContext'
+import { 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,7 +70,7 @@ if (__SSR__) {
     v => (isInSSRComponentSetup = v),
   )
 } else {
-  internalSetCurrentInstance = i => {
+  simpleSetCurrentInstance = i => {
     currentInstance = i
   }
   setInSSRSetupState = v => {
@@ -76,32 +80,10 @@ if (__SSR__) {
 
 export const setCurrentInstance = (instance: GenericComponentInstance) => {
   const prev = currentInstance
-  internalSetCurrentInstance(instance)
-  instance.scope.on()
+  simpleSetCurrentInstance(instance)
+  const prevScope = setCurrentScope(instance.scope)
   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()
+    setCurrentScope(prevScope)
+    simpleSetCurrentInstance(prev)
   }
 }

Diff for: 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))

Diff for: packages/runtime-vapor/__tests__/dom/prop.spec.ts

@@ -17,13 +17,16 @@ import {
   ref,
   simpleSetCurrentInstance,
 } from '@vue/runtime-dom'
+import { setCurrentScope } from '@vue/reactivity'
 
 let removeComponentInstance = NOOP
 beforeEach(() => {
   const instance = new VaporComponentInstance({}, {}, null)
   const prev = currentInstance
   simpleSetCurrentInstance(instance)
+  const prevScope = setCurrentScope(instance.scope)
   removeComponentInstance = () => {
+    setCurrentScope(prevScope)
     simpleSetCurrentInstance(prev)
   }
 })

Diff for: packages/runtime-vapor/src/component.ts

@@ -33,6 +33,7 @@ import {
   pauseTracking,
   proxyRefs,
   resetTracking,
+  setCurrentScope,
   unref,
 } from '@vue/reactivity'
 import { EMPTY_OBJ, invokeArrayFns, isFunction, isString } from '@vue/shared'
@@ -193,6 +194,7 @@ export function createComponent(
 
   const prev = currentInstance
   simpleSetCurrentInstance(instance)
+  const prevScope = setCurrentScope(instance.scope)
   pauseTracking()
 
   if (__DEV__) {
@@ -260,7 +262,8 @@ export function createComponent(
   }
 
   resetTracking()
-  simpleSetCurrentInstance(prev, instance)
+  setCurrentScope(prevScope)
+  simpleSetCurrentInstance(prev)
 
   if (__DEV__) {
     popWarningContext()

Diff for: packages/runtime-vapor/src/componentProps.ts

@@ -23,6 +23,7 @@ import {
 } from '@vue/runtime-dom'
 import { normalizeEmitsOptions } from './componentEmits'
 import { renderEffect } from './renderEffect'
+import { setCurrentScope } from '@vue/reactivity'
 
 export type RawProps = Record<string, () => unknown> & {
   // generated by compiler for :[key]="x" or v-bind="x"
@@ -259,8 +260,10 @@ function resolveDefault(
 ) {
   const prev = currentInstance
   simpleSetCurrentInstance(instance)
+  const prevScope = setCurrentScope(instance.scope)
   const res = factory.call(null, instance.props)
-  simpleSetCurrentInstance(prev, instance)
+  setCurrentScope(prevScope)
+  simpleSetCurrentInstance(prev)
   return res
 }
 

Diff for: packages/runtime-vapor/src/hmr.ts

@@ -13,6 +13,7 @@ import {
   mountComponent,
   unmountComponent,
 } from './component'
+import { setCurrentScope } from '@vue/reactivity'
 
 export function hmrRerender(instance: VaporComponentInstance): void {
   const normalized = normalizeBlock(instance.block)
@@ -21,10 +22,12 @@ export function hmrRerender(instance: VaporComponentInstance): void {
   remove(instance.block, parent)
   const prev = currentInstance
   simpleSetCurrentInstance(instance)
+  const prevScope = setCurrentScope(instance.scope)
   pushWarningContext(instance)
   devRender(instance)
   popWarningContext()
-  simpleSetCurrentInstance(prev, instance)
+  setCurrentScope(prevScope)
+  simpleSetCurrentInstance(prev)
   insert(instance.block, parent, anchor)
 }
 
@@ -38,12 +41,19 @@ export function hmrReload(
   unmountComponent(instance, parent)
   const prev = currentInstance
   simpleSetCurrentInstance(instance.parent)
+  let prevScope: any
+  if (instance.parent !== null) {
+    prevScope = setCurrentScope(instance.parent.scope)
+  }
   const newInstance = createComponent(
     newComp,
     instance.rawProps,
     instance.rawSlots,
     instance.isSingleRoot,
   )
-  simpleSetCurrentInstance(prev, instance.parent)
+  if (instance.parent !== null) {
+    setCurrentScope(prevScope)
+  }
+  simpleSetCurrentInstance(prev)
   mountComponent(newInstance, parent, anchor)
 }

Diff for: packages/runtime-vapor/src/renderEffect.ts

@@ -1,4 +1,9 @@
-import { EffectFlags, type EffectScope, ReactiveEffect } from '@vue/reactivity'
+import {
+  EffectFlags,
+  type EffectScope,
+  ReactiveEffect,
+  setCurrentScope,
+} from '@vue/reactivity'
 import {
   type SchedulerJob,
   currentInstance,
@@ -49,7 +54,7 @@ class RenderEffect extends ReactiveEffect {
   }
 
   callback(): void {
-    const instance = this.i!
+    const instance = this.i
     const scope = this.subs ? (this.subs.sub as EffectScope) : undefined
     // renderEffect is always called after user has registered all hooks
     const hasUpdateHooks = instance && (instance.bu || instance.u)
@@ -58,7 +63,8 @@ class RenderEffect extends ReactiveEffect {
     }
     const prev = currentInstance
     simpleSetCurrentInstance(instance)
-    if (scope) scope.on()
+    let prevScope: any
+    if (scope) prevScope = setCurrentScope(scope)
     if (hasUpdateHooks && instance.isMounted && !instance.isUpdating) {
       instance.isUpdating = true
       instance.bu && invokeArrayFns(instance.bu)
@@ -67,8 +73,8 @@ class RenderEffect extends ReactiveEffect {
     } else {
       this.render()
     }
-    if (scope) scope.off()
-    simpleSetCurrentInstance(prev, instance)
+    if (scope) setCurrentScope(prevScope)
+    simpleSetCurrentInstance(prev)
     if (__DEV__ && instance) {
       startMeasure(instance, `renderEffect`)
     }