{"action":"created","issue":{"url":"https://api.github.com/repos/18F/cg-deck/issues/292","repository_url":"https://api.github.com/repos/18F/cg-deck","labels_url":"https://api.github.com/repos/18F/cg-deck/issues/292/labels{/name}","comments_url":"https://api.github.com/repos/18F/cg-deck/issues/292/comments","events_url":"https://api.github.com/repos/18F/cg-deck/issues/292/events","html_url":"https://github.com/18F/cg-deck/pull/292","id":156756510,"number":292,"title":"Stores use immutable data structures internally","user":{"login":"jeremiak","id":780941,"avatar_url":"https://avatars.githubusercontent.com/u/780941?v=3","gravatar_id":"","url":"https://api.github.com/users/jeremiak","html_url":"https://github.com/jeremiak","followers_url":"https://api.github.com/users/jeremiak/followers","following_url":"https://api.github.com/users/jeremiak/following{/other_user}","gists_url":"https://api.github.com/users/jeremiak/gists{/gist_id}","starred_url":"https://api.github.com/users/jeremiak/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jeremiak/subscriptions","organizations_url":"https://api.github.com/users/jeremiak/orgs","repos_url":"https://api.github.com/users/jeremiak/repos","events_url":"https://api.github.com/users/jeremiak/events{/privacy}","received_events_url":"https://api.github.com/users/jeremiak/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/18F/cg-deck/labels/review-ready","name":"review-ready","color":"fbca04"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2016-05-25T13:53:10Z","updated_at":"2016-05-26T03:07:01Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/18F/cg-deck/pulls/292","html_url":"https://github.com/18F/cg-deck/pull/292","diff_url":"https://github.com/18F/cg-deck/pull/292.diff","patch_url":"https://github.com/18F/cg-deck/pull/292.patch"},"body":"This PR should change all stores so that the underlying data they maintain is kept in immutable data structures using [Immutable.JS](facebook.github.io/immutable-js). This was done in a way that only stores need to be concerned with keeping their data structure in immutable types and that any components that get data from those stores are still working with plain JavaScript objects. I've been casually calling this the \"immutable barrier\". The stores are implemented so that anything returned from a method that begins with `get` such as `getAll()` will convert the immutable data back to regular JS objects before returning them.\r\n\r\nI also changed the API of the BaseStore a bit to make dealing with immutable data a bit easier. For example, I added a [`.push()`](https://github.com/18F/cg-deck/blob/0e57e66b12a5719bad4385ec7509e8de7d99384d/static_src/stores/base_store.js#L31-L37) method which takes an argument, converts it to the respective immutable data type and then creates a new list with that item pushed on and sets that array to be `_data` on the store. It also emits a change event as the underlying data has changed.\r\n\r\nOne of the benefits of using immutability is that we can easily compare nested objects for sameness. This means that we can quickly/cheaply do checks to see if data has changed or not and act accordingly. I added [`dataHasChanged()`](https://github.com/18F/cg-deck/blob/0e57e66b12a5719bad4385ec7509e8de7d99384d/static_src/stores/base_store.js#L53-L56) to the BaseStore which returns a boolean. This gives every store the ability to only emit change events when the data has actually changed. It also provides an interface for components to be able to use the fast equality checking of immutable data types without having to know about them.\r\n\r\nI also added two new merge functions that account for the fact that the data is immutable. A nice benefit is that these merges no longer have unexpected side effects. There are [two new merge APIs](https://github.com/18F/cg-deck/blob/0e57e66b12a5719bad4385ec7509e8de7d99384d/static_src/stores/base_store.js#L106-L141) that have very similar function signatures:\r\n\r\n* `merge(mergeKey, dataToMerge, optionalCallback)`\r\n* `mergeMany(mergeKey, arrayOfDataToMerge, optionalCallback)`\r\n\r\nEach takes three arguments, with the last being an optional callback. The first argument is the key by which to identify similar objects and merge them (generally speaking this is `guid` in our project), the data to merge in, and a callback that is passed `true` or `false` depending on if there was any data that changed. `merge` takes a single object as the data to merge and `mergeMany` takes an array of objects. By default, the callback will call `this.emitChange()` if there is a data change.\r\n\r\nOpen questions (probably for @msecret):\r\n\r\n0. Many of our APIs in this project are promised-based instead of callbacks. For example, all the methods in `cfApi` return promises instead of using callbacks. I wrote the initial version of these merge functions to use callbacks instead of returning a promise, but it might be best to be consistent and use promises throughout. What do you think?\r\n0. Can we remove [`_merge()`](https://github.com/18F/cg-deck/blob/0e57e66b12a5719bad4385ec7509e8de7d99384d/static_src/stores/base_store.js#L77-L90) now that we have these new methods? It doesn't look like its being used at all anymore."},"comment":{"url":"https://api.github.com/repos/18F/cg-deck/issues/comments/221766211","html_url":"https://github.com/18F/cg-deck/pull/292#issuecomment-221766211","issue_url":"https://api.github.com/repos/18F/cg-deck/issues/292","id":221766211,"user":{"login":"coveralls","id":2354108,"avatar_url":"https://avatars.githubusercontent.com/u/2354108?v=3","gravatar_id":"","url":"https://api.github.com/users/coveralls","html_url":"https://github.com/coveralls","followers_url":"https://api.github.com/users/coveralls/followers","following_url":"https://api.github.com/users/coveralls/following{/other_user}","gists_url":"https://api.github.com/users/coveralls/gists{/gist_id}","starred_url":"https://api.github.com/users/coveralls/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/coveralls/subscriptions","organizations_url":"https://api.github.com/users/coveralls/orgs","repos_url":"https://api.github.com/users/coveralls/repos","events_url":"https://api.github.com/users/coveralls/events{/privacy}","received_events_url":"https://api.github.com/users/coveralls/received_events","type":"User","site_admin":false},"created_at":"2016-05-26T03:07:01Z","updated_at":"2016-05-26T03:07:01Z","body":"\n[![Coverage Status](https://coveralls.io/builds/6338875/badge)](https://coveralls.io/builds/6338875)\n\nCoverage remained the same at 72.599% when pulling **d991550ff830e4911c96afdea2948c9d78c0458b on jk-immutable-stores** into **d35b788a74028755137adae0ced7a9eef5df9eaf on staging-alpha**.\n"}}
{"actor":{"display_login":"coveralls"}}