Skip to content

Fix applySnapshot to map<string,boolean> bug#155

Merged
mattiamanzati merged 3 commits intomobxjs:masterfrom
pioh:master
May 29, 2017
Merged

Fix applySnapshot to map<string,boolean> bug#155
mattiamanzati merged 3 commits intomobxjs:masterfrom
pioh:master

Conversation

@pioh
Copy link
Copy Markdown
Contributor

@pioh pioh commented May 29, 2017

Primitive types removes from the map on applySnapshot if key exists.

const Store = types.model('Store', {
  tags: types.map(types.boolean),
})
let store = Store.create({tags: {admin: true}})
console.log(JSON.stringify(getSnapshot(store)))
applySnapshot(store, {tags: {admin: true}})
console.log(JSON.stringify(getSnapshot(store)))
applySnapshot(store, {tags: {admin: true}})
console.log(JSON.stringify(getSnapshot(store)))

Output:
{"tags":{"admin":true}}
{"tags":{}}
{"tags":{"admin":true}}

Primitive types removes from the map on applySnapshot if key exists.

const Store = types.model('Store', {
  tags: types.map(types.boolean),
})
let store = Store.create({tags: {admin: true}})
console.log(JSON.stringify(getSnapshot(store)))
applySnapshot(store, {tags: {admin: true}})
console.log(JSON.stringify(getSnapshot(store)))
applySnapshot(store, {tags: {admin: true}})
console.log(JSON.stringify(getSnapshot(store)))

Output:
{"tags":{"admin":true}}
{"tags":{}}
{"tags":{"admin":true}}
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 93.078% when pulling a12f0d6 on pioh:master into f773d22 on mobxjs:master.

Copy link
Copy Markdown
Contributor

@mattiamanzati mattiamanzati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test for this case, so with futher changes we can be sure to not break the expected behaviour :)

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.06%) to 93.141% when pulling 4bc510a on pioh:master into f773d22 on mobxjs:master.

@pioh
Copy link
Copy Markdown
Contributor Author

pioh commented May 29, 2017

I added one test. Thank you!

@mattiamanzati
Copy link
Copy Markdown
Contributor

Thanks for your effort!

@mattiamanzati mattiamanzati merged commit 72deec3 into mobxjs:master May 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants