refactor: more refinement

Author: yyx990803

packages/reactivity/__tests__/effectScope.spec.ts

@@ -3,16 +3,16 @@ import {
   reactive,
   effect,
   EffectScope,
-  onDispose,
+  onScopeDispose,
   computed,
   ref,
   ComputedRef
 } from '../src'
 
 describe('reactivity/effect/scope', () => {
-  it('should run the passed function once (wrapped by a effect)', () => {
+  it('should run', () => {
     const fnSpy = jest.fn(() => {})
-    new EffectScope(fnSpy)
+    new EffectScope().run(fnSpy)
     expect(fnSpy).toHaveBeenCalledTimes(1)
   })
 
@@ -21,8 +21,13 @@ describe('reactivity/effect/scope', () => {
     expect(scope.effects.length).toBe(0)
   })
 
+  it('should return run value', () => {
+    expect(new EffectScope().run(() => 1)).toBe(1)
+  })
+
   it('should collect the effects', () => {
-    const scope = new EffectScope(() => {
+    const scope = new EffectScope()
+    scope.run(() => {
       let dummy
       const counter = reactive({ num: 0 })
       effect(() => (dummy = counter.num))
@@ -39,7 +44,8 @@ describe('reactivity/effect/scope', () => {
     let dummy, doubled
     const counter = reactive({ num: 0 })
 
-    const scope = new EffectScope(() => {
+    const scope = new EffectScope()
+    scope.run(() => {
       effect(() => (dummy = counter.num))
       effect(() => (doubled = counter.num * 2))
     })
@@ -62,10 +68,11 @@ describe('reactivity/effect/scope', () => {
     let dummy, doubled
     const counter = reactive({ num: 0 })
 
-    const scope = new EffectScope(() => {
+    const scope = new EffectScope()
+    scope.run(() => {
       effect(() => (dummy = counter.num))
       // nested scope
-      new EffectScope(() => {
+      new EffectScope().run(() => {
         effect(() => (doubled = counter.num * 2))
       })
     })
@@ -90,12 +97,13 @@ describe('reactivity/effect/scope', () => {
     let dummy, doubled
     const counter = reactive({ num: 0 })
 
-    const scope = new EffectScope(() => {
+    const scope = new EffectScope()
+    scope.run(() => {
       effect(() => (dummy = counter.num))
       // nested scope
-      new EffectScope(() => {
+      new EffectScope(true).run(() => {
         effect(() => (doubled = counter.num * 2))
-      }, true)
+      })
     })
 
     expect(scope.effects.length).toBe(1)
@@ -114,17 +122,18 @@ describe('reactivity/effect/scope', () => {
     expect(doubled).toBe(12)
   })
 
-  it('able to extend the scope', () => {
+  it('able to run the scope', () => {
     let dummy, doubled
     const counter = reactive({ num: 0 })
 
-    const scope = new EffectScope(() => {
+    const scope = new EffectScope()
+    scope.run(() => {
       effect(() => (dummy = counter.num))
     })
 
     expect(scope.effects.length).toBe(1)
 
-    scope.extend(() => {
+    scope.run(() => {
       effect(() => (doubled = counter.num * 2))
     })
 
@@ -137,25 +146,24 @@ describe('reactivity/effect/scope', () => {
     scope.stop()
   })
 
-  it('can not extend an inactive scope', () => {
+  it('can not run an inactive scope', () => {
     let dummy, doubled
     const counter = reactive({ num: 0 })
 
-    const scope = new EffectScope(() => {
+    const scope = new EffectScope()
+    scope.run(() => {
       effect(() => (dummy = counter.num))
     })
 
     expect(scope.effects.length).toBe(1)
 
     scope.stop()
 
-    scope.extend(() => {
+    scope.run(() => {
       effect(() => (doubled = counter.num * 2))
     })
 
-    expect(
-      '[Vue warn] cannot extend an inactive effect scope.'
-    ).toHaveBeenWarned()
+    expect('[Vue warn] cannot run an inactive effect scope.').toHaveBeenWarned()
 
     expect(scope.effects.length).toBe(1)
 
@@ -164,34 +172,17 @@ describe('reactivity/effect/scope', () => {
     expect(doubled).toBe(undefined)
   })
 
-  it('should fire onStop hook', () => {
-    let dummy = 0
-
-    const scope = new EffectScope(onStop => {
-      onStop(() => (dummy += 1))
-      onStop(() => (dummy += 2))
-    })
-
-    scope.extend(onStop => {
-      onStop(() => (dummy += 4))
-    })
-
-    expect(dummy).toBe(0)
-
-    scope.stop()
-    expect(dummy).toBe(7)
-  })
-
   it('should fire onDispose hook', () => {
     let dummy = 0
 
-    const scope = new EffectScope(() => {
-      onDispose(() => (dummy += 1))
-      onDispose(() => (dummy += 2))
+    const scope = new EffectScope()
+    scope.run(() => {
+      onScopeDispose(() => (dummy += 1))
+      onScopeDispose(() => (dummy += 2))
     })
 
-    scope.extend(() => {
-      onDispose(() => (dummy += 4))
+    scope.run(() => {
+      onScopeDispose(() => (dummy += 4))
     })
 
     expect(dummy).toBe(0)
@@ -208,7 +199,8 @@ describe('reactivity/effect/scope', () => {
     const watchEffectSpy = jest.fn()
 
     let c: ComputedRef
-    const scope = new EffectScope(() => {
+    const scope = new EffectScope()
+    scope.run(() => {
       c = computed(() => {
         computedSpy()
         return r.value + 1

packages/reactivity/src/effectScope.ts

@@ -1,8 +1,6 @@
 import { ReactiveEffect } from './effect'
 import { warn } from './warning'
 
-export type EffectScopeOnStopHook = (fn: (() => void)) => void
-
 let activeEffectScope: EffectScope | undefined
 const effectScopeStack: EffectScope[] = []
 
@@ -11,27 +9,24 @@ export class EffectScope {
   effects: (ReactiveEffect | EffectScope)[] = []
   cleanups: (() => void)[] = []
 
-  constructor(fn?: (onStop: EffectScopeOnStopHook) => any, detached?: boolean) {
+  constructor(detached?: boolean) {
     if (!detached) {
       recordEffectScope(this)
     }
-    if (fn) {
-      this.extend(fn)
-    }
   }
 
-  extend(fn: (onStop: EffectScopeOnStopHook) => any) {
+  run(fn: () => any) {
     if (this.active) {
       try {
         effectScopeStack.push(this)
         activeEffectScope = this
-        fn(cleanup => this.cleanups.push(cleanup))
+        return fn()
       } finally {
         effectScopeStack.pop()
         activeEffectScope = effectScopeStack[effectScopeStack.length - 1]
       }
     } else if (__DEV__) {
-      warn(`cannot extend an inactive effect scope.`)
+      warn(`cannot run an inactive effect scope.`)
     }
   }
 
@@ -58,7 +53,7 @@ export function getCurrentScope() {
   return activeEffectScope
 }
 
-export function onDispose(fn: () => void) {
+export function onScopeDispose(fn: () => void) {
   if (activeEffectScope) {
     activeEffectScope.cleanups.push(fn)
   } else if (__DEV__) {

packages/reactivity/src/index.ts

@@ -51,10 +51,5 @@ export {
   EffectScheduler,
   DebuggerEvent
 } from './effect'
-export {
-  EffectScope,
-  getCurrentScope,
-  onDispose,
-  EffectScopeOnStopHook
-} from './effectScope'
+export { EffectScope, getCurrentScope, onScopeDispose } from './effectScope'
 export { TrackOpTypes, TriggerOpTypes } from './operations'

packages/runtime-core/__tests__/apiWatch.spec.ts

@@ -848,16 +848,16 @@ describe('api: watch', () => {
     render(h(Comp), nodeOps.createElement('div'))
 
     expect(instance!).toBeDefined()
-    expect(instance!.effectScope.effects).toBeInstanceOf(Array)
+    expect(instance!.scope.effects).toBeInstanceOf(Array)
     // includes the component's own render effect AND the watcher effect
-    expect(instance!.effectScope.effects!.length).toBe(2)
+    expect(instance!.scope.effects!.length).toBe(2)
 
     _show!.value = false
 
     await nextTick()
     await nextTick()
 
-    expect(instance!.effectScope.effects![0].active).toBe(false)
+    expect(instance!.scope.effects![0].active).toBe(false)
   })
 
   test('this.$watch should pass `this.proxy` to watch source as the first argument ', () => {

packages/runtime-core/src/apiLifecycle.ts

@@ -37,7 +37,11 @@ export function injectHook(
         // This assumes the hook does not synchronously trigger other hooks, which
         // can only be false when the user does something really funky.
         setCurrentInstance(target)
-        const res = callWithAsyncErrorHandling(hook, target, type, args)
+        const res = target.scope.active
+          ? target.scope.run(() =>
+              callWithAsyncErrorHandling(hook, target, type, args)
+            )
+          : callWithAsyncErrorHandling(hook, target, type, args)
         setCurrentInstance(null)
         resetTracking()
         return res

packages/runtime-core/src/apiWatch.ts

@@ -325,7 +325,7 @@ function doWatch(
     }
   }
 
-  const scope = instance && instance.effectScope
+  const scope = instance && instance.scope
   const effect = new ReactiveEffect(getter, scheduler, scope)
 
   if (__DEV__) {

packages/runtime-core/src/compat/global.ts

@@ -1,5 +1,4 @@
 import {
-  stop,
   isReactive,
   reactive,
   track,
@@ -564,7 +563,7 @@ function installCompatMount(
         }
         delete app._container.__vue_app__
       } else {
-        const { bum, effectScope, um } = instance
+        const { bum, scope, um } = instance
         // beforeDestroy hooks
         if (bum) {
           invokeArrayFns(bum)
@@ -573,8 +572,8 @@ function installCompatMount(
           instance.emit('hook:beforeDestroy')
         }
         // stop effects
-        if (effectScope) {
-          stop(effectScope)
+        if (scope) {
+          scope.stop()
         }
         // unmounted hook
         if (um) {

packages/runtime-core/src/component.ts

@@ -1,6 +1,5 @@
 import { VNode, VNodeChild, isVNode } from './vnode'
 import {
-  ReactiveEffect,
   pauseTracking,
   resetTracking,
   shallowReadonly,
@@ -218,11 +217,6 @@ export interface ComponentInternalInstance {
    * Root vnode of this component's own vdom tree
    */
   subTree: VNode
-  /**
-   * Main update effect
-   * @internal
-   */
-  effect: ReactiveEffect
   /**
    * Bound effect runner to be passed to schedulers
    */
@@ -247,7 +241,7 @@ export interface ComponentInternalInstance {
    * so that they can be automatically stopped on component unmount
    * @internal
    */
-  effectScope: EffectScope
+  scope: EffectScope
   /**
    * cache for proxy access type to avoid hasOwnProperty calls
    * @internal
@@ -452,14 +446,13 @@ export function createComponentInstance(
     root: null!, // to be immediately set
     next: null,
     subTree: null!, // will be set synchronously right after creation
-    effect: null!, // will be set synchronously right after creation
     update: null!, // will be set synchronously right after creation
+    scope: new EffectScope(),
     render: null,
     proxy: null,
     exposed: null,
     exposeProxy: null,
     withProxy: null,
-    effectScope: new EffectScope(),
     provides: parent ? parent.provides : Object.create(appContext.provides),
     accessCache: null!,
     renderCache: [],
@@ -616,23 +609,17 @@ function setupStatefulComponent(
   // 2. call setup()
   const { setup } = Component
   if (setup) {
-    let setupResult: any
     const setupContext = (instance.setupContext =
       setup.length > 1 ? createSetupContext(instance) : null)
 
     currentInstance = instance
     pauseTracking()
-    instance.effectScope.extend(() => {
-      setupResult = callWithErrorHandling(
-        setup,
-        instance,
-        ErrorCodes.SETUP_FUNCTION,
-        [
-          __DEV__ ? shallowReadonly(instance.props) : instance.props,
-          setupContext
-        ]
-      )
-    })
+    const setupResult = instance.scope.run(() =>
+      callWithErrorHandling(setup, instance, ErrorCodes.SETUP_FUNCTION, [
+        __DEV__ ? shallowReadonly(instance.props) : instance.props,
+        setupContext
+      ])
+    )
     resetTracking()
     currentInstance = null
 
@@ -810,9 +797,7 @@ export function finishComponentSetup(
   if (__FEATURE_OPTIONS_API__ && !(__COMPAT__ && skipOptions)) {
     currentInstance = instance
     pauseTracking()
-    instance.effectScope.extend(() => {
-      applyOptions(instance)
-    })
+    instance.scope.run(() => applyOptions(instance))
     resetTracking()
     currentInstance = null
   }