Skip to content

Commit

Permalink
fix(reactivity): revert computed scheduler change
Browse files Browse the repository at this point in the history 
fix #4157
  • Loading branch information
@yyx990803
yyx990803 committed Jul 20, 2021
1 parent bc7f976 commit 33c2fbf
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 304 deletions.
215 changes: 0 additions & 215 deletions packages/reactivity/__tests__/computed.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
ref,
WritableComputedRef,
isReadonly,
setComputedScheduler,
DebuggerEvent,
toRaw,
TrackOpTypes,
Expand Down Expand Up @@ -273,218 +272,4 @@ describe('reactivity/computed', () => {
oldValue: 2
})
})

describe('with scheduler', () => {
// a simple scheduler similar to the main Vue scheduler
const tick = Promise.resolve()
const queue: any[] = []
let queued = false

const schedule = (fn: any) => {
queue.push(fn)
if (!queued) {
queued = true
tick.then(flush)
}
}

const flush = () => {
for (let i = 0; i < queue.length; i++) {
queue[i]()
}
queue.length = 0
queued = false
}

beforeEach(() => {
setComputedScheduler(schedule)
})

afterEach(() => {
setComputedScheduler(undefined)
})

test('should only trigger once on multiple mutations', async () => {
const src = ref(0)
const c = computed(() => src.value)
const spy = jest.fn()
effect(() => {
spy(c.value)
})
expect(spy).toHaveBeenCalledTimes(1)
src.value = 1
src.value = 2
src.value = 3
// not called yet
expect(spy).toHaveBeenCalledTimes(1)
await tick
// should only trigger once
expect(spy).toHaveBeenCalledTimes(2)
expect(spy).toHaveBeenCalledWith(c.value)
})

test('should not trigger if value did not change', async () => {
const src = ref(0)
const c = computed(() => src.value % 2)
const spy = jest.fn()
effect(() => {
spy(c.value)
})
expect(spy).toHaveBeenCalledTimes(1)
src.value = 1
src.value = 2

await tick
// should not trigger
expect(spy).toHaveBeenCalledTimes(1)

src.value = 3
src.value = 4
src.value = 5
await tick
// should trigger because latest value changes
expect(spy).toHaveBeenCalledTimes(2)
})

test('chained computed trigger', async () => {
const effectSpy = jest.fn()
const c1Spy = jest.fn()
const c2Spy = jest.fn()

const src = ref(0)
const c1 = computed(() => {
c1Spy()
return src.value % 2
})
const c2 = computed(() => {
c2Spy()
return c1.value + 1
})

effect(() => {
effectSpy(c2.value)
})

expect(c1Spy).toHaveBeenCalledTimes(1)
expect(c2Spy).toHaveBeenCalledTimes(1)
expect(effectSpy).toHaveBeenCalledTimes(1)

src.value = 1
await tick
expect(c1Spy).toHaveBeenCalledTimes(2)
expect(c2Spy).toHaveBeenCalledTimes(2)
expect(effectSpy).toHaveBeenCalledTimes(2)
})

test('chained computed avoid re-compute', async () => {
const effectSpy = jest.fn()
const c1Spy = jest.fn()
const c2Spy = jest.fn()

const src = ref(0)
const c1 = computed(() => {
c1Spy()
return src.value % 2
})
const c2 = computed(() => {
c2Spy()
return c1.value + 1
})

effect(() => {
effectSpy(c2.value)
})

expect(effectSpy).toHaveBeenCalledTimes(1)
src.value = 2
src.value = 4
src.value = 6
await tick
// c1 should re-compute once.
expect(c1Spy).toHaveBeenCalledTimes(2)
// c2 should not have to re-compute because c1 did not change.
expect(c2Spy).toHaveBeenCalledTimes(1)
// effect should not trigger because c2 did not change.
expect(effectSpy).toHaveBeenCalledTimes(1)
})

test('chained computed value invalidation', async () => {
const effectSpy = jest.fn()
const c1Spy = jest.fn()
const c2Spy = jest.fn()

const src = ref(0)
const c1 = computed(() => {
c1Spy()
return src.value % 2
})
const c2 = computed(() => {
c2Spy()
return c1.value + 1
})

effect(() => {
effectSpy(c2.value)
})

expect(effectSpy).toHaveBeenCalledTimes(1)
expect(effectSpy).toHaveBeenCalledWith(1)
expect(c2.value).toBe(1)

expect(c1Spy).toHaveBeenCalledTimes(1)
expect(c2Spy).toHaveBeenCalledTimes(1)

src.value = 1
// value should be available sync
expect(c2.value).toBe(2)
expect(c2Spy).toHaveBeenCalledTimes(2)
})

test('sync access of invalidated chained computed should not prevent final effect from running', async () => {
const effectSpy = jest.fn()
const c1Spy = jest.fn()
const c2Spy = jest.fn()

const src = ref(0)
const c1 = computed(() => {
c1Spy()
return src.value % 2
})
const c2 = computed(() => {
c2Spy()
return c1.value + 1
})

effect(() => {
effectSpy(c2.value)
})
expect(effectSpy).toHaveBeenCalledTimes(1)

src.value = 1
// sync access c2
c2.value
await tick
expect(effectSpy).toHaveBeenCalledTimes(2)
})

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

// schedule stop
schedule(() => {
c1.effect.stop()
})
// trigger
src.value++
await tick
expect(c1Spy).toHaveBeenCalledTimes(1)
})
})
})
59 changes: 9 additions & 50 deletions packages/reactivity/src/computed.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,6 @@ export interface WritableComputedOptions<T> {
set: ComputedSetter<T>
}

type ComputedScheduler = (fn: () => void) => void
let scheduler: ComputedScheduler | undefined

/**
* Set a scheduler for deferring computed computations
*/
export const setComputedScheduler = (s: ComputedScheduler | undefined) => {
scheduler = s
}

class ComputedRefImpl<T> {
public dep?: Dep = undefined

Expand All @@ -45,55 +35,24 @@ class ComputedRefImpl<T> {
private readonly _setter: ComputedSetter<T>,
isReadonly: boolean
) {
let compareTarget: any
let hasCompareTarget = false
let scheduled = false
this.effect = new ReactiveEffect(getter, (computedTrigger?: boolean) => {
if (scheduler && this.dep) {
if (computedTrigger) {
compareTarget = this._value
hasCompareTarget = true
} else if (!scheduled) {
const valueToCompare = hasCompareTarget ? compareTarget : this._value
scheduled = true
hasCompareTarget = false
scheduler(() => {
if (this.effect.active && this._get() !== valueToCompare) {
triggerRefValue(this)
}
scheduled = false
})
}
// chained upstream computeds are notified synchronously to ensure
// value invalidation in case of sync access; normal effects are
// deferred to be triggered in scheduler.
for (const e of this.dep) {
if (e.computed) {
e.scheduler!(true /* computedTrigger */)
}
}
}
this.effect = new ReactiveEffect(getter, () => {
if (!this._dirty) {
this._dirty = true
if (!scheduler) triggerRefValue(this)
triggerRefValue(this)
}
})
this.effect.computed = true
this[ReactiveFlags.IS_READONLY] = isReadonly
}

private _get() {
if (this._dirty) {
this._dirty = false
return (this._value = this.effect.run()!)
}
return this._value
}

get value() {
trackRefValue(this)
// the computed ref may get wrapped by other proxies e.g. readonly() #3376
return toRaw(this)._get()
const self = toRaw(this)
trackRefValue(self)
if (self._dirty) {
self._dirty = false
self._value = self.effect.run()!
}
return self._value
}

set value(newValue: T) {
Expand Down
1 change: 0 additions & 1 deletion packages/reactivity/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export {
} from './reactive'
export {
computed,
setComputedScheduler,
ComputedRef,
WritableComputedRef,
WritableComputedOptions,
Expand Down
35 changes: 1 addition & 34 deletions packages/runtime-core/__tests__/rendererComponent.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import {
inject,
Ref,
watch,
SetupContext,
computed
SetupContext
} from '@vue/runtime-test'

describe('renderer: component', () => {
Expand Down Expand Up @@ -325,36 +324,4 @@ describe('renderer: component', () => {
expect(serializeInner(root)).toBe(``)
expect(ids).toEqual([ids[0], ids[0] + 1, ids[0] + 2])
})

test('computed that did not change should not trigger re-render', async () => {
const src = ref(0)
const c = computed(() => src.value % 2)
const spy = jest.fn()
const App = {
render() {
spy()
return c.value
}
}

const root = nodeOps.createElement('div')
render(h(App), root)
expect(serializeInner(root)).toBe(`0`)
expect(spy).toHaveBeenCalledTimes(1)

// verify it updates
src.value = 1
src.value = 2
src.value = 3
await nextTick()
expect(serializeInner(root)).toBe(`1`)
expect(spy).toHaveBeenCalledTimes(2) // should only update once

// verify it updates
src.value = 4
src.value = 5
await nextTick()
expect(serializeInner(root)).toBe(`1`)
expect(spy).toHaveBeenCalledTimes(2) // should not need to update
})
})
4 changes: 0 additions & 4 deletions packages/runtime-core/src/scheduler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@ import { ErrorCodes, callWithErrorHandling } from './errorHandling'
import { isArray } from '@vue/shared'
import { ComponentInternalInstance, getComponentName } from './component'
import { warn } from './warning'
import { setComputedScheduler } from '@vue/reactivity'

// set scheduler for computed
setComputedScheduler(queueJob)

export interface SchedulerJob extends Function {
id?: number
Expand Down

0 comments on commit 33c2fbf

Please sign in to comment.