From 002402e5482a3de20252ee756be0f39a8ec50b27 Mon Sep 17 00:00:00 2001 From: WEI Date: Sat, 19 Mar 2022 14:45:25 +0800 Subject: [PATCH 1/3] fix(reactivity): lose activeEffectScope when effectScope(true) --- .../reactivity/__tests__/effectScope.spec.ts | 16 +++++++++++++++- packages/reactivity/src/effectScope.ts | 18 ++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/packages/reactivity/__tests__/effectScope.spec.ts b/packages/reactivity/__tests__/effectScope.spec.ts index 3ce8affe78c..a6029599c90 100644 --- a/packages/reactivity/__tests__/effectScope.spec.ts +++ b/packages/reactivity/__tests__/effectScope.spec.ts @@ -6,7 +6,8 @@ import { onScopeDispose, computed, ref, - ComputedRef + ComputedRef, + getCurrentScope, } from '../src' describe('reactivity/effect/scope', () => { @@ -263,4 +264,17 @@ describe('reactivity/effect/scope', () => { expect(watchSpy).toHaveBeenCalledTimes(1) expect(watchEffectSpy).toHaveBeenCalledTimes(2) }) + + it('getCurrentScope() should not get undefined when new EffectScope(true) inside parentScope.run function ', () => { + const spy = jest.fn() + const parentScope = new EffectScope() + + parentScope.run(() => { + const childScope = new EffectScope(true) + childScope.run(spy) + + expect(getCurrentScope()).not.toBeUndefined() + expect(getCurrentScope() === parentScope).toBe(true) + }) + }) }) diff --git a/packages/reactivity/src/effectScope.ts b/packages/reactivity/src/effectScope.ts index fcacab6e381..771c8b18e35 100644 --- a/packages/reactivity/src/effectScope.ts +++ b/packages/reactivity/src/effectScope.ts @@ -8,13 +8,23 @@ export class EffectScope { effects: ReactiveEffect[] = [] cleanups: (() => void)[] = [] + /** + * only assinged by undetached scope + */ parent: EffectScope | undefined + /** + * record undetached scopes + */ scopes: EffectScope[] | undefined /** * track a child scope's index in its parent's scopes array for optimized * removal */ private index: number | undefined + /** + * activeEffectScope in context not died should be recorded + */ + private aliveEffectScope: EffectScope | undefined constructor(detached = false) { if (!detached && activeEffectScope) { @@ -29,10 +39,12 @@ export class EffectScope { run(fn: () => T): T | undefined { if (this.active) { try { + this.aliveEffectScope = activeEffectScope activeEffectScope = this return fn() } finally { - activeEffectScope = this.parent + activeEffectScope = this.aliveEffectScope + delete this.aliveEffectScope } } else if (__DEV__) { warn(`cannot run an inactive effect scope.`) @@ -40,11 +52,13 @@ export class EffectScope { } on() { + this.aliveEffectScope = activeEffectScope activeEffectScope = this } off() { - activeEffectScope = this.parent + activeEffectScope = this.aliveEffectScope + delete this.aliveEffectScope } stop(fromParent?: boolean) { From 6f947c307c3f572638e3cb4276962cbf5376fa51 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Wed, 23 Mar 2022 10:58:22 +0100 Subject: [PATCH 2/3] test: refactor --- packages/reactivity/__tests__/effectScope.spec.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/reactivity/__tests__/effectScope.spec.ts b/packages/reactivity/__tests__/effectScope.spec.ts index a6029599c90..6adda3d834a 100644 --- a/packages/reactivity/__tests__/effectScope.spec.ts +++ b/packages/reactivity/__tests__/effectScope.spec.ts @@ -7,7 +7,7 @@ import { computed, ref, ComputedRef, - getCurrentScope, + getCurrentScope } from '../src' describe('reactivity/effect/scope', () => { @@ -265,16 +265,16 @@ describe('reactivity/effect/scope', () => { expect(watchEffectSpy).toHaveBeenCalledTimes(2) }) - it('getCurrentScope() should not get undefined when new EffectScope(true) inside parentScope.run function ', () => { - const spy = jest.fn() + it('getCurrentScope() stays valid when running a detached nested EffectScope', () => { const parentScope = new EffectScope() parentScope.run(() => { - const childScope = new EffectScope(true) - childScope.run(spy) + const currentScope = getCurrentScope() + expect(currentScope).toBeDefined() + const detachedScope = new EffectScope(true) + detachedScope.run(() => {}) - expect(getCurrentScope()).not.toBeUndefined() - expect(getCurrentScope() === parentScope).toBe(true) + expect(getCurrentScope()).toBe(currentScope) }) }) }) From 513cc8b66d0e00daa031927d2a9a2e463c89fa85 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Wed, 23 Mar 2022 11:08:39 +0100 Subject: [PATCH 3/3] refactor(effectScope): restore scope after run --- packages/reactivity/src/effectScope.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/reactivity/src/effectScope.ts b/packages/reactivity/src/effectScope.ts index 771c8b18e35..01499c5a78e 100644 --- a/packages/reactivity/src/effectScope.ts +++ b/packages/reactivity/src/effectScope.ts @@ -21,10 +21,6 @@ export class EffectScope { * removal */ private index: number | undefined - /** - * activeEffectScope in context not died should be recorded - */ - private aliveEffectScope: EffectScope | undefined constructor(detached = false) { if (!detached && activeEffectScope) { @@ -38,13 +34,12 @@ export class EffectScope { run(fn: () => T): T | undefined { if (this.active) { + const currentEffectScope = activeEffectScope try { - this.aliveEffectScope = activeEffectScope activeEffectScope = this return fn() } finally { - activeEffectScope = this.aliveEffectScope - delete this.aliveEffectScope + activeEffectScope = currentEffectScope } } else if (__DEV__) { warn(`cannot run an inactive effect scope.`) @@ -52,13 +47,11 @@ export class EffectScope { } on() { - this.aliveEffectScope = activeEffectScope activeEffectScope = this } off() { - activeEffectScope = this.aliveEffectScope - delete this.aliveEffectScope + activeEffectScope = this.parent } stop(fromParent?: boolean) {