Usage with immutable.js

[ghargrove]

@tshaddix

Sorry it took so long to get around to this. I finally got a chance to dig a little deeper into the issue @ericnkatz and I were having here (https://github.com/tshaddix/react-chrome-redux/issues/122).

It does appear to be related to the new shallowDiff stuff but seems to be related to our use of immutable and redux-immutable.

https://github.com/tshaddix/react-chrome-redux/blob/master/src/wrap-store/shallowDiff.js#L15 It looks like oldObj and newObj are expecting regular javascript objects. In our case they're actually receiving immutable Map objects. https://facebook.github.io/immutable-js/docs/#/Map/toJS

If I run the code below everything works as expected

function shallowDiff(oldObj, newObj) {
  var difference = [];

    oldObj = oldObj.toJS()
    newObj = newObj.toJS()

  Object.keys(newObj).forEach(function (key) {

Not exactly sure what the solution to this should be. Any suggestions?

Thanks

[ericnkatz]

@tshaddix any thoughts on this one? We'd love to get an update that can gracefully handle Immutable or at least remove the assumption that everything passed is a vanilla JS Object.

[tshaddix]

@ericnkatz @ghargrove Thanks for diving into this! Yea, this is a bit tough... we made the assumption that the state would be composed of just plain JS objects when we implemented this library and then with the shallow diff algorithm. I'm wondering if the best option for y'all is to use the new custom serializer ability that was just introduced: https://github.com/tshaddix/react-chrome-redux#custom-serialization

You could write a serializer that checks for immutable JS objects and puts them into vanilla JS objects. The vanilla JS object restriction is a restriction of the chrome message passing API which is why we have that requirement. Thoughts?

[ericnkatz]

Will try to take a peak at the custom serialization at the end of the week and let you know how it goes. 👍

[boxi79]

@tshaddix It seems that customized serializer cannot solve this problem.

The initial state passed to content page through chrome messaging is transferred to plain object.

However, the shallowDiff send difference of immutable states, containing these keys:

{key: "_root", value: ArrayMapNode, change: "updated"}
{key: "__ownerID", change: "removed"}
{key: "__hash", change: "removed"}
{key: "__altered", change: "removed"}

shallowDiff compares immutable states directly. These keys cannot update content page store (state that have been transfer to plain object from immutable state through chrome messaging) correctly.

[ericnkatz]

@boxi79 curious what your serializer looks like - haven't had a chance to dig into our implementation for things yet because its not been a priority to update yet but if you have something I can try to apply it to ours and see if we have the same issue.

[boxi79]

@ericnkatz I cannot find out my serializer in my project. Do you also using immutable.js and react-redux to build your extension?

Is it possible to have one so that immutable.js can be applied to background store? I thought it is impossible.

shallowDiff() compare plain object by each key. I dig into this function, and console each keys with immutable state applied:

{key: "_root", value: ArrayMapNode, change: "updated"}
{key: "__ownerID", change: "removed"}
{key: "__hash", change: "removed"}
{key: "__altered", change: "removed"}

So, the diff is something like above.

In order to let patchState in Store.js work fine. The Store's state, which is got from background have to keep immutable object form (having keys: _root, __ownerID,...etc). I tried

const proxyStore = new Store({
  portName: "example",
  deserializer: payload => {
    if (Array.isArray(payload) {  // keep PATCH_STATE_TYPE's payload plain object
      return payload
    }
    return Immutable.fromJS(payload)  // I only want STATE_TYPE's payload transformed to immutable object
  }
})

Therefore, content or popup will got immutable object.

Now we have a immutable state and we want to update this state by patchState(difference) in Store.js. Recall that difference is some thing like:

{key: "_root", value: ArrayMapNode, change: "updated"}
{key: "__ownerID", change: "removed"}
{key: "__hash", change: "removed"}
{key: "__altered", change: "removed"}

, and state is now an immutable state, so these should worked perfectly.

However, const state = Object.assign({}, this.state); in patchState(difference) in Store.js makes something go wrong.

For example:

const obj = Immutable.Map(obj)
const obj2 = Object.assign({}, obj)

obj2 is no longer an immutable object. Despite it keep the immutable object form, it is not an immutable object, but a plain object disguising itself as immutable object.

Comments and discussion welcome. Thanks

P.S.

The instantly solution for me is:

function shallowDiff(oldObj, newObj) {
  var difference = [];

    oldObj = oldObj.toJS()
    newObj = newObj.toJS()
    ......

[ethanroday]

@boxi79, thanks for the in-depth review of the situation! Unfortunately, the reason the custom serialization won't help here is that shallowDiff is run before the custom serialization takes place. So the patched state will never be computed correctly. The custom serializers were introduced to handle cases where Chrome (or whatever browser you're developing for) doesn't automatically serialize objects correctly. One possibility would be to allow a custom shallowDiff function to be provided from client code, but that's getting pretty far into the weeds. Even then, calling toJS() on every state change would be a potentially significant performance hit. At the end of the day, the overarching issue is just that the library makes a lot of assumptions about things being vanilla JS.

[ethanroday]

@boxi79, I'm working on something that will potentially be able to address this - see #153 for details. Basically, in addition to the custom serialization functions, you would also be able to provide custom diffing and patching functions for computing the state differences pre-serialization. So, for this scenario, you could provide custom diffing, serialization, deserialization, and patching functions that handle ImmutableJS stores. Something like the following:

1. Diffing - immutable-js-diff
2. Serialization - transit-immutable-js toJSON
3. Deserialization - transit-immutable-js fromJSON
4. Patching - immutable-js-patch

Give it a try and let me know how it goes!

[chan-dev]

I know this issue has been old but it seems like it still exist in the current version where immutable objects are converted to plain js objects in the proxy store.