Skip to content

Commit

Permalink
fix(reactive): fix the incomplete coverage of runReactions batch mode (
Browse files Browse the repository at this point in the history
  • Loading branch information
janryWang authored Nov 25, 2021
1 parent 88d6f83 commit 91f4ecf
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 5 deletions.
105 changes: 104 additions & 1 deletion packages/reactive/src/__tests__/autorun.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ test('autorun dispose in batch', () => {
expect(handler).toBeCalledTimes(1)
})

test('atom mutate value by computed depend', () => {
test('set value by computed depend', () => {
const obs = observable<any>({})
const comp1 = observable.computed(() => {
return obs.aa?.bb
Expand All @@ -564,6 +564,109 @@ test('atom mutate value by computed depend', () => {
expect(handler).toBeCalledWith(123, 321)
})

test('delete value by computed depend', () => {
const handler = jest.fn()
const obs = observable({
a: {
b: 1,
c: 2,
},
})
const comp1 = observable.computed(() => {
return obs.a?.b
})
const comp2 = observable.computed(() => {
return obs.a?.c
})
autorun(() => {
handler(comp1.value, comp2.value)
})
delete obs.a
expect(handler).toBeCalledTimes(2)
expect(handler).toBeCalledWith(1, 2)
expect(handler).toBeCalledWith(undefined, undefined)
})

test('set Set value by computed depend', () => {
const handler = jest.fn()
const obs = observable({
set: new Set(),
})
const comp1 = observable.computed(() => {
return obs.set.has(1)
})
const comp2 = observable.computed(() => {
return obs.set.size
})
autorun(() => {
handler(comp1.value, comp2.value)
})
obs.set.add(1)
expect(handler).toBeCalledTimes(2)
expect(handler).toBeCalledWith(false, 0)
expect(handler).toBeCalledWith(true, 1)
})

test('delete Set by computed depend', () => {
const handler = jest.fn()
const obs = observable({
set: new Set([1]),
})
const comp1 = observable.computed(() => {
return obs.set.has(1)
})
const comp2 = observable.computed(() => {
return obs.set.size
})
autorun(() => {
handler(comp1.value, comp2.value)
})
obs.set.delete(1)
expect(handler).toBeCalledTimes(2)
expect(handler).toBeCalledWith(true, 1)
expect(handler).toBeCalledWith(false, 0)
})

test('set Map value by computed depend', () => {
const handler = jest.fn()
const obs = observable({
map: new Map(),
})
const comp1 = observable.computed(() => {
return obs.map.has(1)
})
const comp2 = observable.computed(() => {
return obs.map.size
})
autorun(() => {
handler(comp1.value, comp2.value)
})
obs.map.set(1, 1)
expect(handler).toBeCalledTimes(2)
expect(handler).toBeCalledWith(false, 0)
expect(handler).toBeCalledWith(true, 1)
})

test('delete Map by computed depend', () => {
const handler = jest.fn()
const obs = observable({
map: new Map([[1, 1]]),
})
const comp1 = observable.computed(() => {
return obs.map.has(1)
})
const comp2 = observable.computed(() => {
return obs.map.size
})
autorun(() => {
handler(comp1.value, comp2.value)
})
obs.map.delete(1)
expect(handler).toBeCalledTimes(2)
expect(handler).toBeCalledWith(true, 1)
expect(handler).toBeCalledWith(false, 0)
})

test('autorun recollect dependencies', () => {
const obs = observable<any>({
aa: 'aaa',
Expand Down
4 changes: 0 additions & 4 deletions packages/reactive/src/handlers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import {
batchEnd,
batchStart,
bindTargetKeyWithCurrentReaction,
runReactionsFromTargetKey,
} from './reaction'
Expand Down Expand Up @@ -202,7 +200,6 @@ export const baseHandlers: ProxyHandler<any> = {
const newValue = createObservable(target, key, value)
const oldValue = target[key]
target[key] = newValue // use Reflect.set is too slow
batchStart()
if (!hadKey) {
runReactionsFromTargetKey({
target,
Expand All @@ -222,7 +219,6 @@ export const baseHandlers: ProxyHandler<any> = {
type: 'set',
})
}
batchEnd()
return true
},
deleteProperty(target, key) {
Expand Down
2 changes: 2 additions & 0 deletions packages/reactive/src/reaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ export const bindComputedReactions = (reaction: Reaction) => {

export const runReactionsFromTargetKey = (operation: IOperation) => {
let { key, type, target, oldTarget } = operation
batchStart()
notifyObservers(operation)
if (type === 'clear') {
oldTarget.forEach((_: any, key: PropertyKey) => {
Expand All @@ -137,6 +138,7 @@ export const runReactionsFromTargetKey = (operation: IOperation) => {
const newKey = Array.isArray(target) ? 'length' : ITERATION_KEY
runReactions(target, newKey)
}
batchEnd()
}

export const hasRunningReaction = () => {
Expand Down

0 comments on commit 91f4ecf

Please sign in to comment.