ObservableMap replace triggers a change on unchanged values

[elado]

ObservableMap .replace uses .clear, which causes reactions on all values in the map, even if they haven't changed.

import { observable, computed, autorun } from 'mobx'

class Store {
  m = observable.map({ a: 1, b: 2, c: 3 })

  @computed
  get a() {
    console.log('a recompute')
    return this.m.get('a')
  }
}

const store = new Store()

autorun(() => {
  console.log(store.a)
})

console.log('changing b...')

store.m.set('b', 4)

console.log('setting a to same value...')
// won't recompute a
store.m.set('a', 1)

console.log('replacing...')

// store.a will be recomputed even when it wasn't changed
store.m.replace({ a: 1, d: 5 })

https://codesandbox.io/s/61kwwnjvlr

I would expect it to remove missing keys instead of clear and use merge on the rest, and then it won't trigger changes on unchanged values.

https://github.com/mobxjs/mobx/blob/master/src/types/observablemap.ts#L305-L307

[mweststrate]

Good catch! Interested in create a PR? Op wo 15 nov. 2017 om 23:15 schreef Elad Ossadon <[email protected]>:

…

ObservableMap .replace uses .clear, which causes reactions on all values in the map, even if they haven't changed. import { observable, computed, autorun } from 'mobx' class Store { m = observable.map({ a: 1, b: 2, c: 3 }) @computed get a() { console.log('a recompute') return this.m.get('a') } } const store = new Store() autorun(() => { console.log(store.a) }) console.log('changing b...') store.m.set('b', 4) console.log('setting a to same value...')// won't recompute astore.m.set('a', 1) console.log('replacing...') // store.a will be recomputed even when it wasn't changedstore.m.replace({ a: 1, d: 5 }) https://codesandbox.io/s/61kwwnjvlr I would expect it to remove missing keys instead of clear and use merge on the rest, and then it won't trigger changes on unchanged values. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1243>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhIwJiEYi3beTY7qi-E6MNTR39CDAks5s22J8gaJpZM4Qfp6B> .

[elado]

yes! will do in the next few days.

[mweststrate]

Awesome! Thanks in advance Op vr 17 nov. 2017 om 19:52 schreef Elad Ossadon <[email protected]>:

…

yes! will do in the next few days. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1243 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhBaRfeDOhoqYCy7RREwUvJogtQfbks5s3dX2gaJpZM4Qfp6B> .

[elado]

#1247