wip

Author: johnsoncodehk

Diff for: packages/reactivity/__tests__/effectScope.spec.ts

@@ -30,7 +30,8 @@ describe('reactivity/effect/scope', () => {
 
   it('should work w/ active property', () => {
     const scope = effectScope()
-    scope.run(() => 1)
+    const src = computed(() => 1)
+    scope.run(() => src.value)
     expect(scope.active).toBe(true)
     scope.stop()
     expect(scope.active).toBe(false)
@@ -170,13 +171,13 @@ describe('reactivity/effect/scope', () => {
 
     scope.stop()
 
+    expect(getEffectsCount(scope)).toBe(0)
+
     scope.run(() => {
       effect(() => (doubled = counter.num * 2))
     })
 
-    expect('[Vue warn] cannot run an inactive effect scope.').toHaveBeenWarned()
-
-    expect(getEffectsCount(scope)).toBe(0)
+    expect(getEffectsCount(scope)).toBe(1)
 
     counter.num = 7
     expect(dummy).toBe(0)
@@ -359,7 +360,7 @@ describe('reactivity/effect/scope', () => {
     expect(cleanupCalls).toBe(1)
 
     expect(getEffectsCount(scope)).toBe(0)
-    expect(scope.cleanups.length).toBe(0)
+    expect(scope.cleanups).toBe(0)
   })
 })
 

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

@@ -3,12 +3,15 @@ import type { TrackOpTypes, TriggerOpTypes } from './constants'
 import { setupOnTrigger } from './debug'
 import { activeEffectScope } from './effectScope'
 import {
+  type Dependency,
   type Link,
   type Subscriber,
   SubscriberFlags,
   checkDirty,
+  cleanup,
   endTracking,
   link,
+  onCleanup,
   startTracking,
   unlink,
 } from './system'
@@ -36,6 +39,9 @@ export interface DebuggerOptions {
 
 export interface ReactiveEffectOptions extends DebuggerOptions {
   scheduler?: EffectScheduler
+  /**
+   * @deprecated
+   */
   onStop?: () => void
 }
 
@@ -51,26 +57,24 @@ export enum EffectFlags {
   ALLOW_RECURSE = 1 << 7,
   PAUSED = 1 << 8,
   NOTIFIED = 1 << 9,
-  STOP = 1 << 10,
 }
 
-export class ReactiveEffect<T = any> implements ReactiveEffectOptions {
+export class ReactiveEffect<T = any>
+  implements ReactiveEffectOptions, Dependency, Subscriber
+{
   // Subscriber
   deps: Link | undefined = undefined
   depsTail: Link | undefined = undefined
   flags: number = 0
+  cleanups: number = 0
 
   // Dependency
   subs: Link | undefined = undefined
   subsTail: Link | undefined = undefined
 
-  /**
-   * @internal
-   */
-  cleanup?: () => void = undefined
-
-  onStop?: () => void
+  // dev only
   onTrack?: (event: DebuggerEvent) => void
+  // dev only
   onTrigger?: (event: DebuggerEvent) => void
 
   // @ts-expect-error
@@ -80,13 +84,13 @@ export class ReactiveEffect<T = any> implements ReactiveEffectOptions {
     if (fn !== undefined) {
       this.fn = fn
     }
-    if (activeEffectScope && activeEffectScope.active) {
+    if (activeEffectScope) {
       link(this, activeEffectScope)
     }
   }
 
   get active(): boolean {
-    return !(this.flags & EffectFlags.STOP)
+    return this.deps !== undefined
   }
 
   pause(): void {
@@ -122,12 +126,10 @@ export class ReactiveEffect<T = any> implements ReactiveEffectOptions {
   }
 
   run(): T {
-    let flags = this.flags
-    if (flags & EffectFlags.STOP) {
-      // stopped during cleanup
-      return this.fn()
+    const cleanups = this.cleanups
+    if (cleanups) {
+      cleanup(this, cleanups)
     }
-    cleanupEffect(this)
     const prevSub = activeSub
     setActiveSub(this)
     startTracking(this)
@@ -143,7 +145,7 @@ export class ReactiveEffect<T = any> implements ReactiveEffectOptions {
       }
       setActiveSub(prevSub)
       endTracking(this)
-      flags = this.flags
+      const flags = this.flags
       if (
         (flags & (SubscriberFlags.Recursed | EffectFlags.ALLOW_RECURSE)) ===
         (SubscriberFlags.Recursed | EffectFlags.ALLOW_RECURSE)
@@ -155,17 +157,15 @@ export class ReactiveEffect<T = any> implements ReactiveEffectOptions {
   }
 
   stop(): void {
-    const flags = this.flags
-    if (!(flags & EffectFlags.STOP)) {
-      this.flags = flags | EffectFlags.STOP
-      startTracking(this)
-      endTracking(this)
-      cleanupEffect(this)
-      this.onStop && this.onStop()
-
-      if (this.subs !== undefined) {
-        unlink(this.subs)
-      }
+    const sub = this.subs
+    const cleanups = this.cleanups
+    if (sub !== undefined) {
+      unlink(sub)
+    }
+    startTracking(this)
+    endTracking(this)
+    if (cleanups) {
+      cleanup(this, cleanups)
     }
   }
 
@@ -202,6 +202,15 @@ export function effect<T = any>(
 
   const e = new ReactiveEffect(fn)
   if (options) {
+    const onStop = options.onStop
+    if (onStop) {
+      delete options.onStop
+      const stop = e.stop.bind(e)
+      e.stop = () => {
+        stop()
+        onStop()
+      }
+    }
     extend(e, options)
   }
   try {
@@ -286,8 +295,9 @@ export function resetTracking(): void {
  * an active effect.
  */
 export function onEffectCleanup(fn: () => void, failSilently = false): void {
-  if (activeSub instanceof ReactiveEffect) {
-    activeSub.cleanup = fn
+  const e = activeSub
+  if (e instanceof ReactiveEffect) {
+    onCleanup(e, () => cleanupEffect(e, fn))
   } else if (__DEV__ && !failSilently) {
     warn(
       `onEffectCleanup() was called when there was no active effect` +
@@ -296,18 +306,14 @@ export function onEffectCleanup(fn: () => void, failSilently = false): void {
   }
 }
 
-function cleanupEffect(e: ReactiveEffect) {
-  const cleanup = e.cleanup
-  if (cleanup !== undefined) {
-    e.cleanup = undefined
-    // run cleanup without active effect
-    const prevSub = activeSub
-    activeSub = undefined
-    try {
-      cleanup()
-    } finally {
-      activeSub = prevSub
-    }
+function cleanupEffect(e: ReactiveEffect, fn: () => void) {
+  // run cleanup without active effect
+  const prevSub = activeSub
+  activeSub = undefined
+  try {
+    fn()
+  } finally {
+    activeSub = prevSub
   }
 }
 

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

@@ -3,7 +3,11 @@ import {
   type Dependency,
   type Link,
   type Subscriber,
+  cleanup,
+  endTracking,
   link,
+  onCleanup,
+  startTracking,
   unlink,
 } from './system'
 import { warn } from './warning'
@@ -15,28 +19,20 @@ export class EffectScope implements Subscriber, Dependency {
   deps: Link | undefined = undefined
   depsTail: Link | undefined = undefined
   flags: number = 0
+  cleanups: number = 0
 
   // Dependency
   subs: Link | undefined = undefined
   subsTail: Link | undefined = undefined
 
-  /**
-   * @internal
-   */
-  cleanups: (() => void)[] = []
-  /**
-   * @internal
-   */
-  cleanupsLength = 0
-
   constructor(detached = false) {
     if (!detached && activeEffectScope) {
       link(this, activeEffectScope)
     }
   }
 
   get active(): boolean {
-    return !(this.flags & EffectFlags.STOP)
+    return this.deps !== undefined
   }
 
   notify(): void {}
@@ -70,35 +66,25 @@ export class EffectScope implements Subscriber, Dependency {
   }
 
   run<T>(fn: () => T): T | undefined {
-    const flags = this.flags
-    if (!(flags & EffectFlags.STOP)) {
-      const prevEffectScope = activeEffectScope
-      try {
-        activeEffectScope = this
-        return fn()
-      } finally {
-        activeEffectScope = prevEffectScope
-      }
-    } else if (__DEV__) {
-      warn(`cannot run an inactive effect scope.`)
+    const prevEffectScope = activeEffectScope
+    try {
+      activeEffectScope = this
+      return fn()
+    } finally {
+      activeEffectScope = prevEffectScope
     }
   }
 
   stop(): void {
-    const flags = this.flags
-    if (!(flags & EffectFlags.STOP)) {
-      this.flags = flags | EffectFlags.STOP
-      while (this.deps !== undefined) {
-        unlink(this.deps)
-      }
-      const l = this.cleanupsLength
-      for (let i = 0; i < l; i++) {
-        this.cleanups[i]()
-      }
-      this.cleanupsLength = 0
-      if (this.subs !== undefined) {
-        unlink(this.subs)
-      }
+    const sub = this.subs
+    const cleanups = this.cleanups
+    if (sub !== undefined) {
+      unlink(sub)
+    }
+    startTracking(this)
+    endTracking(this)
+    if (cleanups) {
+      cleanup(this, cleanups)
     }
   }
 }
@@ -142,7 +128,7 @@ export function setCurrentScope(
  */
 export function onScopeDispose(fn: () => void, failSilently = false): void {
   if (activeEffectScope) {
-    activeEffectScope.cleanups[activeEffectScope.cleanupsLength++] = fn
+    onCleanup(activeEffectScope, fn)
   } else if (__DEV__ && !failSilently) {
     warn(
       `onScopeDispose() is called when there is no active effect scope` +