Calculate shouldExpandNode on JSONNestedNode.render

[gnestor]

Closes #60

[gnestor]

Here's a test component:

import React from 'react';
import JSONTree from 'react-json-tree';

export class JSONFilterableTree extends React.Component {

  constructor(props) {
    super(props);
    this.state = {
      filter: ''
    };
  }

  render() {
    let data = this.props.data;
    if (this.state.filter) {
      data = Object.keys(this.props.data).reduce((result, key) => {
        if (JSON.stringify(this.props.data[key]).includes(this.state.filter)) result[key] = this.props.data[key];
        return result;
      }, {});
    }
    return (
      <div
        style={{
          position: 'relative'
        }}
      >
        <input
          value={this.state.filter}
          onChange={(event) => {
            this.setState({filter: event.target.value});
          }}
          style={{
            position: 'absolute',
            top: 0,
            right: 0,
            width: '33%',
            maxWidth: 150,
            zIndex: 10,
            fontSize: 13,
            paddingHorizontal: 4,
            paddingVertical: 2
          }}
          type="text"
          placeholder="Filter..."
        />
        <JSONTree data={data} />
      </div>
    );
  }

}

[alexkuz]

I think it's better to move that to componentWillReceiveProps, like this:

const getStateFromProps = props => ({
  ...  // calculate `expanded`
});

...

constructor(props) {
  super(props);
  this.state = getStateFromProps(props);
}

componentWillReceiveProps(nextProps) {
  if (['shouldExpandNode', 'isCircular', 'keyPath', 'data', 'level'].find(k => nextProps[k] !== this.props[k]) {
    this.setState(getStateFromProps(nextProps));
  }
}

[gnestor]

Ya, better to keep it DRY. I tested and this is working 👍

[gnestor]

@alexkuz I made the changes you suggested. I think this is ready to merge 👍

[cephirothdy2j]

+1 on this; I'm waiting on the same fix. Will this be merged and released any time soon?

[alexkuz]

Yes, I'll take a look soon, sorry for delay.

[alexkuz]

What is this for? Seems like it's not used anywhere.

[alexkuz]

@gnestor Thank you! Everything is fine, except for that seemingly redundant state value.

[alexkuz]

Oh, wait, it was there before. Nevermind then :)

[zalmoxisus]

Could the breaking changes from this pr be moved under a flag (prop)?

As mentioned in #66, it breaks tree inspection for both Redux DevTools Inspector and Log Monitor:

[gnestor]

What if we changed the following:

componentWillReceiveProps(nextProps) {
  if (['shouldExpandNode', 'isCircular', 'keyPath', 'data', 'level'].find(k =>
    nextProps[k] !== this.props[k])
  ) {
    this.setState(getStateFromProps(nextProps));
  }
}

to:

componentWillReceiveProps(nextProps) {
  if (getStateFromProps(nextProps) !== getStateFromProps(this.props)) {
    this.setState(getStateFromProps(nextProps));
  }
}

In its current form, componentWillReceiveProps is over-eager to recalculate expanded state, but this change should correct that (because it appears that the only value that is changing in the above screencap is data). @zalmoxisus Can you test whether this fixes the issue in redux-devtools-inspector? And whether it has any effect on performance?

[zalmoxisus]

@gnestor it doesn't help, but if (false) helps 😂

You can clone redux-devtools-inspector and run yarn && yarn start to open the demo.

Not sure if solving that specific issue when an action is selected (as in my screenshot) would be enough. According to #66, users got used that, even when selecting a different action, the state tree remains expanded. If so, we could just add a prop to enable the introduced updates. @alexkuz what do you think? Do we need these updates in Inspector?

[alexkuz]

@zalmoxisus try v0.10.3 (or RDI v0.11.2), it should work now.

As @gnestor proposed, now it just checks if state.expanded was changed.

[zalmoxisus]

@alexkuz yes, it works. Thanks a lot for the fix!