Skip to content

Commit

Permalink
Try fix vuejs#10236
Browse files Browse the repository at this point in the history
  • Loading branch information
johnsoncodehk committed Feb 7, 2024
1 parent 65eedb1 commit 3710635
Show file tree
Hide file tree
Showing 10 changed files with 33 additions and 87 deletions.
21 changes: 0 additions & 21 deletions packages/reactivity/__tests__/computed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,21 +122,6 @@ describe('reactivity/computed', () => {
expect(getter2).toHaveBeenCalledTimes(2)
})

it('should no longer update when stopped', () => {
const value = reactive<{ foo?: number }>({})
const cValue = computed(() => value.foo)
let dummy
effect(() => {
dummy = cValue.value
})
expect(dummy).toBe(undefined)
value.foo = 1
expect(dummy).toBe(1)
cValue.effect.stop()
value.foo = 2
expect(dummy).toBe(1)
})

it('should support setter', () => {
const n = ref(1)
const plusOne = computed({
Expand Down Expand Up @@ -218,12 +203,6 @@ describe('reactivity/computed', () => {
expect(isReadonly(z.value.a)).toBe(false)
})

it('should expose value when stopped', () => {
const x = computed(() => 1)
x.effect.stop()
expect(x.value).toBe(1)
})

it('debug: onTrack', () => {
let events: DebuggerEvent[] = []
const onTrack = vi.fn((e: DebuggerEvent) => {
Expand Down
16 changes: 0 additions & 16 deletions packages/reactivity/__tests__/deferredComputed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,20 +136,4 @@ describe('deferred computed', () => {
c2.value
expect(effectSpy).toHaveBeenCalledTimes(2)
})

test('should not compute if deactivated before scheduler is called', () => {
const c1Spy = vi.fn()
const src = ref(0)
const c1 = computed(() => {
c1Spy()
return src.value % 2
})
effect(() => c1.value)
expect(c1Spy).toHaveBeenCalledTimes(1)

c1.effect.stop()
// trigger
src.value++
expect(c1Spy).toHaveBeenCalledTimes(1)
})
})
2 changes: 1 addition & 1 deletion packages/reactivity/__tests__/effectScope.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,8 @@ describe('reactivity/effect/scope', () => {
r.value++
c!.value
await nextTick()
expect(computedSpy).toHaveBeenCalledTimes(3)
// should not trigger anymore
expect(computedSpy).toHaveBeenCalledTimes(2)
expect(watchSpy).toHaveBeenCalledTimes(1)
expect(watchEffectSpy).toHaveBeenCalledTimes(2)
})
Expand Down
4 changes: 3 additions & 1 deletion packages/reactivity/src/computed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ export class ComputedRefImpl<T> {
),
)
this.effect.computed = this
this.effect.active = this._cacheable = !isSSR
// TODO: How SSR affect computed?
// this.effect.active = this._cacheable = !isSSR
this._cacheable = !isSSR
this[ReactiveFlags.IS_READONLY] = isReadonly
}

Expand Down
22 changes: 15 additions & 7 deletions packages/reactivity/src/effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export type DebuggerEventExtraInfo = {
export let activeEffect: ReactiveEffect | undefined

export class ReactiveEffect<T = any> {
active = true
deps: Dep[] = []

/**
Expand All @@ -39,7 +38,6 @@ export class ReactiveEffect<T = any> {
*/
allowRecurse?: boolean

onStop?: () => void
// dev only
onTrack?: (event: DebuggerEvent) => void
// dev only
Expand Down Expand Up @@ -105,9 +103,6 @@ export class ReactiveEffect<T = any> {

run() {
this._dirtyLevel = DirtyLevels.NotDirty
if (!this.active) {
return this.fn()
}
let lastShouldTrack = shouldTrack
let lastEffect = activeEffect
try {
Expand All @@ -123,6 +118,19 @@ export class ReactiveEffect<T = any> {
shouldTrack = lastShouldTrack
}
}
}

export class ReactiveSideEffect<T = any> extends ReactiveEffect<T> {
active = true

onStop?: () => void

run() {
if (!this.active) {
return this.fn()
}
return super.run()
}

stop() {
if (this.active) {
Expand Down Expand Up @@ -177,7 +185,7 @@ export interface ReactiveEffectOptions extends DebuggerOptions {

export interface ReactiveEffectRunner<T = any> {
(): T
effect: ReactiveEffect
effect: ReactiveSideEffect
}

/**
Expand All @@ -198,7 +206,7 @@ export function effect<T = any>(
fn = (fn as ReactiveEffectRunner).effect.fn
}

const _effect = new ReactiveEffect(fn, NOOP, () => {
const _effect = new ReactiveSideEffect(fn, NOOP, () => {
if (_effect.dirty) {
_effect.run()
}
Expand Down
9 changes: 6 additions & 3 deletions packages/reactivity/src/effectScope.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ReactiveEffect } from './effect'
import type { ReactiveEffect, ReactiveSideEffect } from './effect'
import { warn } from './warning'

let activeEffectScope: EffectScope | undefined
Expand All @@ -11,7 +11,7 @@ export class EffectScope {
/**
* @internal
*/
effects: ReactiveEffect[] = []
effects: (ReactiveEffect | ReactiveSideEffect)[] = []
/**
* @internal
*/
Expand Down Expand Up @@ -82,7 +82,10 @@ export class EffectScope {
if (this._active) {
let i, l
for (i = 0, l = this.effects.length; i < l; i++) {
this.effects[i].stop()
const effect = this.effects[i]
if ('stop' in effect) {
effect.stop()
}
}
for (i = 0, l = this.cleanups.length; i < l; i++) {
this.cleanups[i]()
Expand Down
1 change: 1 addition & 0 deletions packages/reactivity/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export {
pauseScheduling,
resetScheduling,
ReactiveEffect,
ReactiveSideEffect,
type ReactiveEffectRunner,
type ReactiveEffectOptions,
type EffectScheduler,
Expand Down
35 changes: 0 additions & 35 deletions packages/runtime-core/__tests__/apiSetupHelpers.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import {
type ComponentInternalInstance,
type ComputedRef,
type SetupContext,
Suspense,
computed,
createApp,
defineComponent,
getCurrentInstance,
Expand Down Expand Up @@ -419,38 +417,5 @@ describe('SFC <script setup> helpers', () => {
expect(uids.one.before).toBe(uids.one.after)
expect(uids.two.before).toBe(uids.two.after)
})

test('should teardown in-scope effects', async () => {
let resolve: (val?: any) => void
const ready = new Promise(r => {
resolve = r
})

let c: ComputedRef

const Comp = defineComponent({
async setup() {
let __temp: any, __restore: any
;[__temp, __restore] = withAsyncContext(() => Promise.resolve())
__temp = await __temp
__restore()

c = computed(() => {})
// register the lifecycle after an await statement
onMounted(resolve)
return () => ''
},
})

const app = createApp(() => h(Suspense, () => h(Comp)))
const root = nodeOps.createElement('div')
app.mount(root)

await ready
expect(c!.effect.active).toBe(true)

app.unmount()
expect(c!.effect.active).toBe(false)
})
})
})
6 changes: 5 additions & 1 deletion packages/runtime-core/__tests__/apiWatch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,11 @@ describe('api: watch', () => {
await nextTick()
await nextTick()

expect(instance!.scope.effects[0].active).toBe(false)
for (const effect of instance!.scope.effects) {
if ('active' in effect) {
expect(effect.active).toBe(false)
}
}
})

test('this.$watch should pass `this.proxy` to watch source as the first argument ', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/runtime-core/src/apiWatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {
type ComputedRef,
type DebuggerOptions,
type EffectScheduler,
ReactiveEffect,
ReactiveSideEffect,
ReactiveFlags,

Check failure on line 6 in packages/runtime-core/src/apiWatch.ts

View workflow job for this annotation

GitHub Actions / lint-and-test-dts

Member 'ReactiveFlags' of the import declaration should be sorted alphabetically
type Ref,
getCurrentScope,
Expand Down Expand Up @@ -392,7 +392,7 @@ function doWatch(
scheduler = () => queueJob(job)
}

const effect = new ReactiveEffect(getter, NOOP, scheduler)
const effect = new ReactiveSideEffect(getter, NOOP, scheduler)

const scope = getCurrentScope()
const unwatch = () => {
Expand Down

0 comments on commit 3710635

Please sign in to comment.