Skip to content

Commit 1be76b8

Browse files
committed
perf(doura): invalidate api snapshot on view dirty
1 parent 8b25bd5 commit 1be76b8

5 files changed

Lines changed: 70 additions & 20 deletions

File tree

packages/doura/src/core/__tests__/views.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -973,17 +973,21 @@ describe('createView', () => {
973973

974974
const child = modelMgr.getModel(childModel)
975975
const parent = modelMgr.getModel(parentModel)
976+
const parentInternal = (modelMgr as any)._models.get('parent')
976977

977978
// Initial: parent view reads child.count = 0
978979
const api1 = parent.$getApi()
979980
expect(api1.childCount).toBe(0)
981+
expect(parentInternal._api).toBe(api1)
980982

981983
// Mutate child
982984
child.inc()
985+
expect(parentInternal._api).toBe(null)
983986
await nextTick()
984987

985988
// Parent's $getApi() should return updated view reflecting child.count = 1
986989
const api2 = parent.$getApi()
990+
expect(api2).not.toBe(api1)
987991
expect(api2.childCount).toBe(1)
988992
})
989993
})

packages/doura/src/core/model.ts

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { warn } from '../warning'
1111
import {
1212
view as reactiveView,
1313
type View,
14+
type ViewOptions,
1415
effectScope,
1516
type EffectScope,
1617
draft,
@@ -207,7 +208,6 @@ export class ModelInternal<M extends Model = Model> {
207208
models: Record<string, ModelInstance<any>>
208209
modelProxies: Record<string, ModelInstance<any>>
209210
viewInstances: ViewExt[] = []
210-
private _modelViews: ViewExt[] = []
211211
accessContext: AccessContext
212212

213213
stateRef: {
@@ -250,6 +250,7 @@ export class ModelInternal<M extends Model = Model> {
250250
this.isolate = this.isolate.bind(this)
251251
this.getApi = this.getApi.bind(this)
252252
this._update = this._update.bind(this)
253+
this._invalidateApiSnapshot = this._invalidateApiSnapshot.bind(this)
253254
this._flushQueryListeners = this._flushQueryListeners.bind(this)
254255

255256
this.options = model
@@ -367,17 +368,6 @@ export class ModelInternal<M extends Model = Model> {
367368
}
368369

369370
getApi() {
370-
// Invalidate cache if any model-defined view is dirty (e.g. from
371-
// cross-model dependency changes that bypass _setState).
372-
if (this._api !== null) {
373-
for (let i = 0; i < this._modelViews.length; i++) {
374-
if (this._modelViews[i].dirty) {
375-
this._api = null
376-
break
377-
}
378-
}
379-
}
380-
381371
if (this._api === null) {
382372
const data = Object.create(this._apiProto)
383373
this._api = Object.assign(data, this._currentState, this.views)
@@ -431,7 +421,7 @@ export class ModelInternal<M extends Model = Model> {
431421
})
432422
}
433423

434-
createView(viewFn: (s: any) => any): ViewExt {
424+
createView(viewFn: (s: any) => any, options: ViewOptions = {}): ViewExt {
435425
let view!: ViewExt
436426
this.effectScope.run(() => {
437427
view = reactiveView(() => {
@@ -458,7 +448,7 @@ export class ModelInternal<M extends Model = Model> {
458448
} finally {
459449
this.accessContext = oldCtx
460450
}
461-
}) as ViewExt
451+
}, options) as ViewExt
462452
})
463453

464454
view.getSnapshot = () => {
@@ -535,7 +525,7 @@ export class ModelInternal<M extends Model = Model> {
535525
destroy() {
536526
// reset props
537527
this._destroyed = true
538-
this._api = null
528+
this._invalidateApiSnapshot()
539529
this._lastDraftToSnapshot = new WeakMap()
540530

541531
this._currentState = null
@@ -572,8 +562,12 @@ export class ModelInternal<M extends Model = Model> {
572562
return this._queryHashScope
573563
}
574564

575-
private _setState(newState: ModelStateFromModel<M>) {
565+
private _invalidateApiSnapshot() {
576566
this._api = null
567+
}
568+
569+
private _setState(newState: ModelStateFromModel<M>) {
570+
this._invalidateApiSnapshot()
577571
this._currentState = newState
578572
this.stateValue = this.stateRef.value
579573
}
@@ -683,8 +677,9 @@ export class ModelInternal<M extends Model = Model> {
683677
for (const viewName of Object.keys(views)) {
684678
this._cacheAccess(viewName, AccessTypes.VIEW)
685679
const viewFn = views[viewName]
686-
const view = this.createView(viewFn)
687-
this._modelViews.push(view)
680+
const view = this.createView(viewFn, {
681+
onDirty: this._invalidateApiSnapshot,
682+
})
688683
Object.defineProperty(this.views, viewName, {
689684
configurable: true,
690685
enumerable: true,

packages/doura/src/reactivity/__tests__/view.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,52 @@ describe('reactivity/view', () => {
4242
expect(getter).toHaveBeenCalledTimes(2)
4343
})
4444

45+
it('should not call onDirty on creation or first read', () => {
46+
const value = draft({ foo: 0 })
47+
const onDirty = jest.fn()
48+
const cValue = view(() => value.foo, { onDirty })
49+
50+
expect(onDirty).not.toHaveBeenCalled()
51+
expect(cValue.value).toBe(0)
52+
expect(onDirty).not.toHaveBeenCalled()
53+
})
54+
55+
it('should call onDirty once when becoming dirty', () => {
56+
const value = draft({ foo: 0 })
57+
const onDirty = jest.fn()
58+
const cValue = view(() => value.foo, { onDirty })
59+
60+
expect(cValue.value).toBe(0)
61+
value.foo = 1
62+
expect(onDirty).toHaveBeenCalledTimes(1)
63+
64+
value.foo = 2
65+
expect(onDirty).toHaveBeenCalledTimes(1)
66+
67+
expect(cValue.value).toBe(2)
68+
value.foo = 3
69+
expect(onDirty).toHaveBeenCalledTimes(2)
70+
})
71+
72+
it('should call onDirty before dependent effects rerun', () => {
73+
const value = draft({ foo: 0 })
74+
const events: string[] = []
75+
const cValue = view(() => value.foo, {
76+
onDirty: () => {
77+
events.push('dirty')
78+
},
79+
})
80+
81+
effect(() => {
82+
events.push(`effect:${cValue.value}`)
83+
})
84+
expect(events).toEqual(['effect:0'])
85+
86+
events.length = 0
87+
value.foo = 1
88+
expect(events).toEqual(['dirty', 'effect:1'])
89+
})
90+
4591
it('should trigger effect', () => {
4692
const value = draft<{ foo?: number }>({})
4793
const cValue = view(() => value.foo)

packages/doura/src/reactivity/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export {
77
markRaw,
88
markStrict,
99
} from './common'
10-
export { View, view, ViewGetter } from './view'
10+
export { View, view, ViewGetter, ViewOptions } from './view'
1111
export { draft, snapshot, watch, resetDraftChildren } from './draft'
1212
export { ReactiveEffect, pauseTracking, resetTracking } from './effect'
1313
export { EffectScope, effectScope } from './effectScope'

packages/doura/src/reactivity/view.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export interface View<T = any> {
1313

1414
export interface ViewOptions {
1515
disableCache?: boolean
16+
onDirty?: () => void
1617
}
1718

1819
export type ViewGetter<T> = (...args: any[]) => T
@@ -30,10 +31,14 @@ export class ViewImpl<T> {
3031

3132
private _cacheable: boolean
3233

33-
constructor(getter: ViewGetter<T>, { disableCache = false }: ViewOptions) {
34+
constructor(
35+
getter: ViewGetter<T>,
36+
{ disableCache = false, onDirty }: ViewOptions
37+
) {
3438
this.effect = new ReactiveEffect(getter, () => {
3539
if (!this.dirty) {
3640
this.dirty = true
41+
onDirty?.()
3742
triggerView(this)
3843
}
3944
})

0 commit comments

Comments
 (0)