added immutable-js support via toMutable function

[cchamberlain]

Love this dev-tool, IMHO it is the most useful one yet.

Only problem for me is that I'm converting my state tree to use immutable-js and as soon as it hits a node in state tree that is immutable, the chart will not show the contents of the node or any of its children, instead it just says "error": "not-serializable". It also throws a bunch of warnings in the console about using size instead of length property on immutable objects is deprecated and will throw in future releases of immutable-js. I wrote a simple recursive function to walk the state tree, check if any of the objects are immutable, and if so convert them to plain objects. If it finds no immutables in state it will simply return equivalent simple state.

The toMutable function can be found here

What I see with the vanilla library when I try to inspect an immutable object:

What I see with the toMutable support added:

Given immutable-js is very commonly used, I'm submitting this as a pull request. If you are not looking to support immutable, you may disregard and I'll keep the alternate version published at https://www.npmjs.com/package/redux-devtools-chart-monitor-immutable

Again, thanks for the awesome work!

[romseguy]

Thanks for the heads up, I'm not sure whether the perf implications are very impactful, but with large state trees it could be nice to have a prop we pass to ChartMonitor so this is an opt-in behaviour, what do you think?

Although I know there are other areas that could be improved regarding performance (e.g map2tree and d3 stuff), so deep conversion of the state map to plain JS is probably not much of a concern.

[cchamberlain]

Great points, I reworked it as an opt-in under prop "hasImmutables", defaulting to false. While I was in there I also added an option for "invertTheme", defaulted to false that inverts the base00 - base07 colors if set to true.

So now for this configuration:

I get this result:

Again, feel free to take it or leave it, but I think I've removed all the performance impact.

[cchamberlain]

Constructor extraneous here.

[romseguy]

Looks like tomutable needs improvement, I got a stack overflow with hasImmutables turned on. My state doesn't use Immutable.js but that's not the problem of course. The state I used is not so big, just 2 big arrays with 40-50 children. Good idea for invertTheme btw.

[cchamberlain]

Thanks, I hadn't run into errors yet but I've only been using for my narrow use case. I will add tests to that project and get it fixed up.

[gaearon]

Why not iterate over plain objects or immutable collections in a generic way? This seems simpler than traversing the whole tree and creating new objects.

Here is an example of how we started supporting Immutable objects in Redux DevTools: reduxjs/redux-devtools#79. Notice how we use for of loop if typeof obj[Symbol.iterator] === 'function'.

cc @FredyC who implemented this.

[cchamberlain]

Thanks @gaearon, that sounds like the best approach. I started to make an attempt at this but don't have anything yet. It seems like the logic that needs to be refactored for that is in renderChart function of d3-state-visualizer. I may get around to playing with this over the weekend if nothing happens with it by then.

[romseguy]

The redux state (a map) is traversed by map2tree to be converted into the tree structure required by the D3 chart :

{
  name: 'state',
  children: [{
    name: 'substate',
    children: [...]
  }, ...
  ]
}

It might make sense to use another library than map2tree altogether because I wrote this one without caring about perf and immutability. Maybe there's a better alternative out there. Or, we can rewrite it a bit with for of loops and type checks so it supports Immutable.js and is more performant.

[romseguy]

I'll be adding immutable.js support and tests to map2tree today.

[romseguy]

I haven't tested with a real word Redux state that is using Immutable.js but as far as the tests go, it works.
I'm just checking for toJS existence and calling that if there is... https://github.com/romseguy/map2tree/blob/master/src/index.js#L43-L44

[cchamberlain]

Thanks, it looks like this is a more efficient way than I originally proposed (down to 1 recursive instead of 2). I'll get this setup in our project and let you know if there are any issues.

I think @gaearon may have been hinting instead of using toJS, we could instead iterate the immutables/non-immutables directly and build the UI from their values. Speculating a bit but I think that would require modifying the tree function from d3 directly to intake a state object along with a transform function to recursively pull the tree relevant values from it (immutable or not).

Much larger undertaking but I may try it if I get some time. I haven't looked at the d3 code yet, so not really sure how complicated this would be. If anyone has advice, I'm all ears.

[ajmajma]

I find this did not work for me, I am assuming it's because I am using immutable-redux's combine reducers. I was able to fix it by just checking if it was immutable.isIterable(), and then a toJS if so - I made a fork to fool around with it. https://github.com/ajmajma/redux-devtools-chart-monitor . This works for me, but I am trying to enforce my entire state to be immutable.

[romseguy]

@ajmajma I don't use ImmutableJS myself so I was waiting for some feedback before publishing map2tree with immutable support to npm. Everything is already pushed to the repo master branch though. Can you manually test your use case with the latest commit? Or should I tag and publish a new version of map2tree as unstable or something?

[cchamberlain]

@romseguy If you publish a tagged release of map2tree I'd be happy to test it with react 15.

[romseguy]

@cchamberlain cool, just published map2tree@1.4.0

[cchamberlain]

I'm not seeing any issues with map2tree@1.4.0 in my app. I can see all the nested immutables fine. Thanks!

[cchamberlain]

Going to close this for now, feel free to roll in any of the color inversion stuff or let me know if you'd like me to put it in a separate pull request.