diff --git a/packages/reactivity/__tests__/effectScope.spec.ts b/packages/reactivity/__tests__/effectScope.spec.ts index e34723cc..a9e4ab0c 100644 --- 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 e88abfc2..db1ebdea 100644 --- 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) {