refactor: simplify for size

Author: yyx990803

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 () => {

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) {