refactor: simplify for size

diff --git a/packages/reactivity/__tests__/effectScope.spec.ts b/packages/reactivity/__tests__/effectScope.spec.ts
index e34723cc3d104d4aea368fb7f03ca42ac4d25a3f..a9e4ab0c79ff3b325bac30018b7919c4ed518503 100644 (file)
--- a/packages/reactivity/__tests__/effectScope.spec.ts
+++ b/packages/reactivity/__tests__/effectScope.spec.ts
@@ -78,8 +78,8 @@ describe('reactivity/effect/scope', () => {
     })
 
     expect(scope.effects.length).toBe(1)
-    expect(scope.scopes.length).toBe(1)
-    expect(scope.scopes[0]).toBeInstanceOf(EffectScope)
+    expect(scope.scopes!.length).toBe(1)
+    expect(scope.scopes![0]).toBeInstanceOf(EffectScope)
 
     expect(dummy).toBe(0)
     counter.num = 7
@@ -195,9 +195,9 @@ describe('reactivity/effect/scope', () => {
   it('should derefence child scope from parent scope after stopping child scope (no memleaks)', async () => {
     const parent = new EffectScope()
     const child = parent.run(() => new EffectScope())!
-    expect(parent.scopes.includes(child)).toBe(true)
+    expect(parent.scopes!.includes(child)).toBe(true)
     child.stop()
-    expect(parent.scopes.includes(child)).toBe(false)
+    expect(parent.scopes!.includes(child)).toBe(false)
   })
 
   it('test with higher level APIs', async () => {

diff --git a/packages/reactivity/src/effectScope.ts b/packages/reactivity/src/effectScope.ts
index e88abfc20cff9f0bf85cc84087be05de294ce939..db1ebdea518ae125bac52411d44d119962ef779f 100644 (file)
--- a/packages/reactivity/src/effectScope.ts
+++ b/packages/reactivity/src/effectScope.ts
@@ -8,21 +8,25 @@ export class EffectScope {
   active = true
   effects: ReactiveEffect[] = []
   cleanups: (() => void)[] = []
+
   parent: EffectScope | undefined
-  private children: EffectScope[] | undefined
-  private parentIndex: number | undefined
+  scopes: EffectScope[] | undefined
+  /**
+   * track a child scope's index in its parent's scopes array for optimized
+   * removal
+   */
+  private index: number | undefined
 
   constructor(detached = false) {
-    if (!detached) {
-      this.recordEffectScope()
+    if (!detached && activeEffectScope) {
+      this.parent = activeEffectScope
+      this.index =
+        (activeEffectScope.scopes || (activeEffectScope.scopes = [])).push(
+          this
+        ) - 1
     }
   }
 
-  get scopes(): EffectScope[] {
-    if (!this.children) this.children = []
-    return this.children
-  }
-
   run<T>(fn: () => T): T | undefined {
     if (this.active) {
       try {
@@ -50,33 +54,25 @@ export class EffectScope {
     }
   }
 
-  stop() {
+  stop(fromParent?: boolean) {
     if (this.active) {
       this.effects.forEach(e => e.stop())
-      this.children?.forEach(e => e.stop())
       this.cleanups.forEach(cleanup => cleanup())
-      this.parent?.derefChildScope(this)
+      if (this.scopes) {
+        this.scopes.forEach(e => e.stop(true))
+      }
+      // nested scope, dereference from parent to avoid memory leaks
+      if (this.parent && !fromParent) {
+        // optimized O(1) removal
+        const last = this.parent.scopes!.pop()
+        if (last && last !== this) {
+          this.parent.scopes![this.index!] = last
+          last.index = this.index!
+        }
+      }
       this.active = false
     }
   }
-
-  private recordEffectScope() {
-    const parent = activeEffectScope
-    if (parent && parent.active) {
-      this.parent = parent
-      this.parentIndex = parent.scopes.push(this) - 1
-    }
-  }
-
-  private derefChildScope(scope: EffectScope) {
-    // reuse the freed index by moving the last array entry
-    const last = this.scopes.pop()
-    if (last && last !== scope) {
-      const childIndex = scope.parentIndex!
-      this.scopes[childIndex] = last
-      last.parentIndex = childIndex
-    }
-  }
 }
 
 export function effectScope(detached?: boolean) {