Skip to content

Commit eb8ad49

Browse files
christianalfonichristianalfoni
committed
fix(overmind): make derived check every mutation to flag as dirty
1 parent 61a2dae commit eb8ad49

File tree

8 files changed

+154
-35
lines changed

8 files changed

+154
-35
lines changed

‎packages/node_modules/overmind/src/derived.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,37 @@ describe('Derived', () => {
6666
expect(app.state.upperFoo).toBe('BAR2')
6767
expect(renderCount).toBe(1)
6868
})
69+
test('should not require flush to flag as dirty', () => {
70+
expect.assertions(1)
71+
const changeFoo: Action = ({ map }) =>
72+
map(({ state }) => state.upperFoo)
73+
.mutate((state) => (state.foo = 'bar2'))
74+
.run(({ state }) => {
75+
expect(state.upperFoo).toBe('BAR2')
76+
})
77+
78+
const derived: Derive<string> = (state: State) => state.foo.toUpperCase()
79+
80+
const config = {
81+
state: {
82+
foo: 'bar',
83+
upperFoo: derived,
84+
},
85+
actions: {
86+
changeFoo,
87+
},
88+
}
89+
type Config = TConfig<{
90+
state: {
91+
foo: string
92+
upperFoo: string
93+
}
94+
actions: typeof config.actions
95+
}>
96+
type Action<Input = void, Output = any> = TAction<Input, Output, Config>
97+
type Derive<T> = TDerive<T, Config>
98+
99+
const app = new App(config)
100+
app.actions.changeFoo()
101+
})
69102
})

‎packages/node_modules/overmind/src/derived.ts‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,16 @@ export default class Derived {
2121
this.value = this.cb(proxyStateTree.get())
2222
this.isDirty = false
2323
this.paths = proxyStateTree.clearPathsTracking(trackId)
24-
if (this.proxyStateTreeListener) {
25-
this.proxyStateTreeListener.update(this.paths)
26-
} else {
24+
if (!this.proxyStateTreeListener) {
2725
this.proxyStateTreeListener = proxyStateTree.addMutationListener(
28-
this.paths,
29-
(flushId) => {
30-
eventHub.emitAsync(EventType.DERIVED_DIRTY, {
31-
path,
32-
flushId,
33-
})
34-
this.isDirty = true
26+
(mutation, flushId) => {
27+
if (this.paths.has(mutation.path)) {
28+
eventHub.emitAsync(EventType.DERIVED_DIRTY, {
29+
path,
30+
flushId,
31+
})
32+
this.isDirty = true
33+
}
3534
}
3635
)
3736
}

‎packages/node_modules/overmind/src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ export default class App<
438438
return this.proxyStateTree.clearPathsTracking(id, cb)
439439
}
440440
addMutationListener(paths, cb) {
441-
return this.proxyStateTree.addMutationListener(paths, cb)
441+
return this.proxyStateTree.addFlushListener(paths, cb)
442442
}
443443
createReactionFactory(prefix: string) {
444444
const reactions = []

‎packages/node_modules/overmind/src/reaction.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class Reaction {
1818
this.proxyStateTree.clearPathsTracking(trackId)
1919
)[0]
2020

21-
this.proxyStateTreeListener = this.proxyStateTree.addMutationListener(
21+
this.proxyStateTreeListener = this.proxyStateTree.addFlushListener(
2222
(mutations, flushId) => {
2323
for (let mutationIndex in mutations) {
2424
const mutation = mutations[mutationIndex]

‎packages/node_modules/proxy-state-tree/README.md‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ function render () {
102102
return tree.clearPathsTracking(trackId)
103103
}
104104

105-
const listener = tree.addMutationListener(render(), (flushId) => {
105+
const listener = tree.addFlushListener(render(), (flushId) => {
106106
// Runs when mutations matches paths passed in
107107

108108
// Whenever mutations affecting these paths occurs
@@ -126,7 +126,7 @@ listener.dispose()
126126

127127
Here we combine the tracked paths with the mutations performed to see if this components, computed or whatever indeed needs to run again, doing a new **startPathsTracking** and **clearPathsTracking**.
128128

129-
You can optionally declare a global listener which will be informed by all mutations:
129+
You can optionally declare a global listener which will be informed by all mutation flushes:
130130

131131
```js
132132
import ProxyStateTree from 'proxy-state-tree'
@@ -137,7 +137,7 @@ const tree = new ProxyStateTree({
137137
})
138138
const state = tree.get()
139139

140-
const listener = tree.addMutationListener((mutations, flushId) => {
140+
const listener = tree.addFlushListener((mutations, flushId) => {
141141
/*
142142
[{
143143
method: "set",
@@ -154,6 +154,33 @@ tree.flush()
154154
listener.dispose()
155155
```
156156

157+
In addition to this you can also listen to each individual mutation:
158+
159+
```js
160+
import ProxyStateTree from 'proxy-state-tree'
161+
162+
const tree = new ProxyStateTree({
163+
foo: 'bar',
164+
bar: 'baz'
165+
})
166+
const state = tree.get()
167+
168+
const listener = tree.addMutationListener((mutation) => {
169+
/*
170+
{
171+
method: "set",
172+
path: "foo",
173+
args: ["bar2"]
174+
}
175+
*/
176+
})
177+
178+
tree.startMutationTracking()
179+
state.foo = 'bar2'
180+
tree.clearMutationTracking()
181+
listener.dispose()
182+
```
183+
157184
## Dynamic state values
158185

159186
If you insert a function into the state tree it will be called when accessed. The function is passed the **proxy-state-tree** instance and the path of where the function lives in the tree.

‎packages/node_modules/proxy-state-tree/src/index.test.ts‎

Lines changed: 59 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,52 @@ describe('TRACKING', () => {
3737
tree.clearPathsTracking(trackId)
3838
expect(hasCalledCb).toBe(true)
3939
})
40+
test('should allow tracking by flush', () => {
41+
let reactionCount = 0
42+
const tree = new ProxyStateTree({
43+
foo: 'bar',
44+
})
45+
const state = tree.get()
46+
47+
const trackId = tree.startPathsTracking()
48+
state.foo
49+
const paths = tree.clearPathsTracking(trackId)
50+
51+
tree.addFlushListener(paths, (flushId) => {
52+
reactionCount++
53+
expect(flushId).toBe(0)
54+
})
55+
tree.startMutationTracking()
56+
state.foo = 'bar2'
57+
tree.clearMutationTracking()
58+
tree.flush()
59+
expect(reactionCount).toBe(1)
60+
})
61+
test('should allow tracking by mutation', () => {
62+
let reactionCount = 0
63+
const tree = new ProxyStateTree({
64+
foo: 'bar',
65+
})
66+
const state = tree.get()
67+
68+
const trackId = tree.startPathsTracking()
69+
state.foo
70+
tree.clearPathsTracking(trackId)
71+
72+
tree.addMutationListener((mutation, flushId) => {
73+
reactionCount++
74+
expect(mutation).toEqual({
75+
method: 'set',
76+
path: 'foo',
77+
args: ['bar2'],
78+
})
79+
expect(flushId).toBe(1)
80+
})
81+
tree.startMutationTracking()
82+
state.foo = 'bar2'
83+
tree.clearMutationTracking()
84+
expect(reactionCount).toBe(1)
85+
})
4086
})
4187

4288
describe('OBJECTS', () => {
@@ -217,7 +263,7 @@ describe('ARRAYS', () => {
217263
}
218264

219265
if (!listener) {
220-
tree.addMutationListener(paths, trackPaths)
266+
tree.addFlushListener(paths, trackPaths)
221267
}
222268

223269
iterations++
@@ -452,7 +498,7 @@ describe('FUNCTIONS', () => {
452498
})
453499
const paths = tree.clearPathsTracking(trackId)
454500

455-
tree.addMutationListener(paths, (flushId) => {
501+
tree.addFlushListener(paths, (flushId) => {
456502
reactionCount++
457503
expect(flushId).toBe(0)
458504
})
@@ -475,7 +521,7 @@ describe('REACTIONS', () => {
475521
const trackId = tree.startPathsTracking()
476522
state.foo // eslint-disable-line
477523
const paths = tree.clearPathsTracking(trackId)
478-
tree.addMutationListener(paths, (flushId) => {
524+
tree.addFlushListener(paths, (flushId) => {
479525
reactionCount++
480526
expect(flushId).toBe(0)
481527
})
@@ -491,7 +537,7 @@ describe('REACTIONS', () => {
491537
foo: 'bar',
492538
})
493539
const state = tree.get()
494-
tree.addMutationListener((mutations, flushId) => {
540+
tree.addFlushListener((mutations, flushId) => {
495541
expect(mutations).toEqual([
496542
{
497543
method: 'set',
@@ -518,7 +564,7 @@ describe('REACTIONS', () => {
518564
renderCount++
519565
return tree.clearPathsTracking(trackId)
520566
}
521-
const listener = tree.addMutationListener(render(), () => {
567+
const listener = tree.addFlushListener(render(), () => {
522568
listener.update(render())
523569
})
524570
tree.startMutationTracking()
@@ -538,7 +584,7 @@ describe('REACTIONS', () => {
538584
state.foo // eslint-disable-line
539585
state.bar // eslint-disable-line
540586
const paths = tree.clearPathsTracking(trackId)
541-
tree.addMutationListener(paths, () => {
587+
tree.addFlushListener(paths, () => {
542588
reactionCount++
543589
})
544590
tree.startMutationTracking()
@@ -562,7 +608,7 @@ describe('REACTIONS', () => {
562608
}
563609
return tree.clearPathsTracking(trackId)
564610
}
565-
const listener = tree.addMutationListener(render(), () => {
611+
const listener = tree.addFlushListener(render(), () => {
566612
listener.update(render())
567613
})
568614
tree.startMutationTracking()
@@ -586,7 +632,7 @@ describe('REACTIONS', () => {
586632
}
587633
return tree.clearPathsTracking(trackId)
588634
}
589-
const listener = tree.addMutationListener(render(), () => {
635+
const listener = tree.addFlushListener(render(), () => {
590636
listener.dispose()
591637
})
592638
tree.startMutationTracking()
@@ -679,7 +725,7 @@ describe('ITERATIONS', () => {
679725
if (mutationListener) {
680726
mutationListener.update(paths)
681727
} else {
682-
mutationListener = tree.addMutationListener(paths, update)
728+
mutationListener = tree.addFlushListener(paths, update)
683729
}
684730

685731
if (runCount === 1) {
@@ -728,7 +774,7 @@ describe('ITERATIONS', () => {
728774
expect(paths).toEqual(
729775
new Set(['items', 'items.0', 'items.0.title', 'items.1', 'items.1.title'])
730776
)
731-
tree.addMutationListener(paths, () => {
777+
tree.addFlushListener(paths, () => {
732778
reactionCount++
733779
})
734780
tree.startMutationTracking()
@@ -758,7 +804,7 @@ describe('ITERATIONS', () => {
758804
expect(paths).toEqual(
759805
new Set(['items', 'items.0', 'items.0.title', 'items.1', 'items.1.title'])
760806
)
761-
tree.addMutationListener(paths, () => {
807+
tree.addFlushListener(paths, () => {
762808
reactionCount++
763809
})
764810
tree.startMutationTracking()
@@ -777,7 +823,7 @@ describe('ITERATIONS', () => {
777823
state.items.map((item) => item)
778824
const paths = tree.clearPathsTracking(trackId)
779825
expect(paths).toEqual(new Set(['items', 'items.0', 'items.1']))
780-
tree.addMutationListener(paths, () => {
826+
tree.addFlushListener(paths, () => {
781827
reactionCount++
782828
})
783829
tree.startMutationTracking()
@@ -825,7 +871,7 @@ describe('PRODUCTION', () => {
825871
}
826872
)
827873

828-
tree.addMutationListener(new Set(['items.0.title']), () => {})
874+
tree.addFlushListener(new Set(['items.0.title']), () => {})
829875

830876
tree.startMutationTracking()
831877
tree.get().items[0].title = 'foo1'

‎packages/node_modules/proxy-state-tree/src/index.ts‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ class ProxyStateTree {
2424
private status: Set<STATUS> = new Set()
2525
private currentFlushId: number = 0
2626
private completedTrackingCallbacks = []
27+
private mutationCallbacks = []
2728
private proxy: any
2829
constructor(private state: object, private options: Options = {}) {
2930
if (!isPlainObject(state)) {
@@ -38,6 +39,12 @@ class ProxyStateTree {
3839

3940
this.proxy = proxify(this, state)
4041
}
42+
private addMutation(mutation: Mutation) {
43+
this.currentMutations.push(mutation)
44+
for (let cb of this.mutationCallbacks) {
45+
cb(mutation, this.currentFlushId + 1)
46+
}
47+
}
4148
get() {
4249
return this.proxy
4350
}
@@ -138,9 +145,16 @@ class ProxyStateTree {
138145

139146
return pathSet
140147
}
141-
addMutationListener(cb: (mutations: Mutation[], flushId: number) => void)
142-
addMutationListener(initialPaths: Set<string>, cb: (flushId: number) => void)
143-
addMutationListener() {
148+
addMutationListener(cb: (mutation: Mutation, flushId: number) => void) {
149+
const index = this.mutationCallbacks.push(cb) - 1
150+
151+
return () => {
152+
this.mutationCallbacks.splice(index, 1)
153+
}
154+
}
155+
addFlushListener(cb: (mutations: Mutation[], flushId: number) => void)
156+
addFlushListener(initialPaths: Set<string>, cb: (flushId: number) => void)
157+
addFlushListener() {
144158
if (arguments.length === 1) {
145159
const cb = arguments[0]
146160
const globalDependencies = this.globalDependencies

‎packages/node_modules/proxy-state-tree/src/proxify.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ function createArrayProxy(tree, value, path) {
7777
) {
7878
ensureMutationTrackingIsEnabled(tree, nestedPath)
7979
return (...args) => {
80-
tree.currentMutations.push({
80+
tree.addMutation({
8181
method: prop,
8282
path: path,
8383
args: args,
@@ -99,7 +99,7 @@ function createArrayProxy(tree, value, path) {
9999
ensureMutationTrackingIsEnabled(tree, nestedPath)
100100
ensureValueDosntExistInStateTreeElsewhere(value)
101101

102-
tree.currentMutations.push({
102+
tree.addMutation({
103103
method: 'set',
104104
path: nestedPath,
105105
args: [value],
@@ -147,7 +147,7 @@ function createObjectProxy(tree, value, path) {
147147
tree.objectChanges.add(path)
148148
}
149149

150-
tree.currentMutations.push({
150+
tree.addMutation({
151151
method: 'set',
152152
path: nestedPath,
153153
args: [value],
@@ -166,7 +166,7 @@ function createObjectProxy(tree, value, path) {
166166
tree.objectChanges.add(path)
167167
}
168168

169-
tree.currentMutations.push({
169+
tree.addMutation({
170170
method: 'unset',
171171
path: nestedPath,
172172
args: [],

0 commit comments

Comments
 (0)