1.0.0.8 broke serialize state setting

[camwest]

See screenshot:

I still have states serialization selected:

Since window.devToolsExtension now takes deserializeState as an option, maybe we should add serializeState as an option there too?

[zalmoxisus]

Due to changes in #60, the immutable data is broken on serialization. I reverted it and just published 1.0.0.9, it will be available on Chrome Store in about an hour. Thanks for the report!

I'll keep this issue open till we find a solution.

[kolodny]

hmm, can you give me a minimal case that breaks in jsan that I can try to work with? thanks

[zalmoxisus]

@kolodny,

import assert from 'assert';
import immutable from 'immutable';
import jsan from 'jsan';

assert.equal(jsan.stringify(immutable.List(), null, null, true), '[]');

Actually the result {"size":0,"_origin":0,"_capacity":0,"_level":5,"_root":{"$jsan":"u"},"_tail":{"$jsan":"u"},"__ownerID":{"$jsan":"u"},"__hash":{"$jsan":"u"},"__altered":false}}},"error":{"$jsan":"u"}} is almost what we get in console for console.log(immutable.List()), and it is still a mystery for me how JSON.stringify serializes it to [].

[kolodny]

it appears that List has a prototype chain of IndexedIterable -> Collection -> Object and Collection has it's own toJSON() method. Should be an easy fix

[zalmoxisus]

I see, thanks for the explanation. We should also check whether it takes nested data like

immutable.Map({
  someList: immutable.List(),
  someMap: immutable.Map()
})

[kolodny]

This is pretty doable by offloading toJSON() to whatever is being serialized, however you lose the advantage of working with functions and other types. I would advise checking for a .toJS method and then using that for stringify, something like

- message.payload = stringify(message.payload, null, null, true);
- if (message.action !== '') message.action = stringify(message.action, null, null, true);

+ var payload = message.payload && message.payload.toJS ? message.payload.toJS() : message.payload;
+ var action = message.action && message.action.toJS ? message.action.toJS() : message.action;
+ message.payload = stringify(message.payload, null, null, true);
+ if (message.action !== '') message.action = stringify(message.action, null, null, true);

I'll still patch jsan to properly handle an object with a toJSON(), however I still think this solution is better in this instance

[zalmoxisus]

It wouldn't be possible to use .toJS in this case as usually only a part of the object is Immutable, which comes from reducers. As in kolodny/jsan#4.

[rhalff]

I've also started to experience the root: properties today, not the weird serialization of the immutable objects though, I'm using seamless-immutable maybe that's why.

Anyway, I found a recent change in react-json-tree is probably related.
I've tested the 0.5.3 version and version 0.5.6, something changed with it's keyName property, see:

alexkuz/react-json-tree@f98e568

The LogMonitor is using it here:

https://github.com/gaearon/redux-devtools-log-monitor/blob/master/src/LogMonitorEntry.js#L43
https://github.com/gaearon/redux-devtools-log-monitor/blob/master/src/LogMonitorEntryAction.js#L25

If you test the latest version of react-json-tree you'll see the keyName is not being set anymore and is set to root.

[zalmoxisus]

@rhalff, yes, related to gaearon/redux-devtools-log-monitor#39 and alexkuz/react-json-tree#32. I'll revert this as well. Thanks for the report.

[kolodny]

I just pushed 3.1.1 which should fix this, in particualr to use it with immutable you would do the same as this test does https://github.com/kolodny/jsan/blob/master/test/end-to-end.js#L107-L125 lines 119-120 are where the replacer function that you would need is. Let me know if this fixes it for you

[rhalff]

.10 works like a charm again, thanks.

[zalmoxisus]

@kolodny, thanks for the changes! I like the idea of nested .toJS.

Is this processing suppose to be slower than just jsan.stringify(obj)? If so, I will let users to choose which type of serialization to use (say "Regular" or "Deep").

[kolodny]

yeah this puts it in the same performance class as the other options, as opposed to without where it's ~ 3x faster for most common use cases. Putting it behind a flag would make the most sense