Add tree view example

[gaearon]

This adds a performant tree view example by a popular request.
Answers https://stackoverflow.com/questions/34981924/performance-issues-with-a-tree-structure-and-shouldcomponentupdate-in-react-re.

[erikras]

This is fantastic. 👍

[CrocoDillon]

Agreed, this example covers so much 😄

I like how you break “convention” by putting action types in the action file instead of a constants file which makes sense. Also your root reducer doesn’t care about action types since it does duck typing instead (check for nodeId on the action).

Also many people think that you should have few connected components, for example only root. And here you have 1000 connected components! 😁

I think examples like this by influential people like you are good, shows the community boilerplate can be reduced by not sticking to convention when it doesn’t make sense for your use-case.

[stephenwf]

No dispatch needed?

[erikras]

The connect() call on line 60 binds the actions to dispatch.

[stephenwf]

Ah, that makes sense! Not seen that being used before.

[ghost]

Nice example, I'm wondering tho how an action affecting the entire state would be handled? For example removing a node, which in turn affects the entire tree?

I need to get my head around this for optimization purpose, I did encounter performance issues with a somewhat big tree of data.

Say for example I have a <Selected /> component that stores added items from a list, when added it shows up in the Selection component, but in the list shows as "added", and vice-versa. How would that need to be handled in the tree-view example?

Really appreciate you taking the time doing an example for this by the way, thanks!

[tamagokun]

A great enhancement to this example would be removing node and moving nodes.

[gaearon]

I'm happy to accept a PR for removing node. Would you like to work on this?

[gaearon]

Nice example, I'm wondering tho how an action affecting the entire state would be handled? For example removing a node, which in turn affects the entire tree?

Why does removing affect the entire tree? Seems like it would only affect the removed node's parent (and, of course, its children, but they would just get unmounted).

[erikras]

There are three possible ways, I see, for removing a node, assuming that you want to remove all of its children from the store...

1. Actually keep the data in a tree structure in the store, so removing a node object automatically removes all its children, which it contained.
2. Maintain the flat structure and give them string ids like '1.5.23.1' so that the reducer can just remove all ids that start with the removed node's id.
3. Maintain the flat structure and depth-first iterate on the removal through all the child nodes.

I did #1 in my redux-form library, which added much complication. I think I'd recommend #3 for this example.

[bramdevries]

Here's a method I used to remove nodes on my own project (uses Immutable.js) but using the same flat structure:

const parentIds = state.get('tree').reduce((result, item) => {
    if (item.get('children').includes(action.id)) {
        result.push(item.get('id'));
    }
    return result;
}, []);

parentIds.forEach(id => {
    const path = ['tree', String(id), 'children'];
    path.push(state.getIn(path).findIndex(i => i === action.id));
    state = state.removeIn(path);
});

return state.removeIn(['tree', String(action.id)]);

Basically what happens is that we run through all nodes and get the id's of where the removed node is a child, then we loop over all these nodes and remove the reference. Finally we remove the actual node.

This is not 100% perfect though, for example. if you remove a node that still has children it won't remove those nodes.

[ghost]

Maybe my data structure is what's confusing me here. I looked back at the example and noticed the multiple connect()ed components instead of only one HOC. I always thoughts this was best practice, so basically what I've been doing is connect the entire state to the top-most container, and then pass along state and dispatch to the children as props:

class FilesDownload extends Component {

  render() {
    return (
      <div className="files-download">
        <div className="downloads">
          <div className="downloads__selection">
            <Selection {...this.props} />
          </div>
          <div className="downloads__list">
            <Filters {...this.props} />
            <Results {...this.props} />
          </div>
        </div>
      </div>
    );
  }

}

function select(state) {
  return state;
}

// Wrap the component to inject dispatch and state into it
export default connect(select)(FilesDownload);

Here's an example Gif of the actual app, you can see the delay when I first click on the "add to selection" button:

EDIT: as you can see too, once the first action happens, then things run smoothly. I know I'm missing something here but can't get my head around it. I need to keep digging 😉

[erikras]

Node Removal PR #1274.

[ghost]

Ah yes thanks @erikras I will look at the PR for reference. Merci 😄

[gaearon]

I always thoughts this was best practice, so basically what I've been doing is connect the entire state to the top-most container, and then pass along state and dispatch to the children as props:

Yeah, we no longer recommend single top container in favor of a more gradual approach. See the updated https://medium.com/@dan_abramov/smart-and-dumb-components-7ca2f9a7c7d0#.49376my94.

[ghost]

Yes thanks for the reminder, as I tweeted earlier (tweet which you "liked") I'm slowing getting the hang of this. It's really starting to make sense now with Presentational and Container Components.

Perhaps, with my example above, I should have a connect() container for each selectable items (CAD Drawing, Install Drawings etc.), what's your thoughts on this?

[gaearon]

Perhaps, with my example above, I should have a connect() container for each selectable items (CAD Drawing, Install Drawings etc.), what's your thoughts on this?

Yes, I think this can work fine.

[ghost]

👍 Merci beaucoup mon ami !

[fhelwanger]

Just wondering... couldn't this action creator receive just nodeId and generate childId internally?

This way, the component doesn't need to call createNode just to get the new id.

[gaearon]

This is what I tried at first but it made the reducer a little bit awkward. It's hard to say which approach is cleaner until the app grows large enough and more use cases are implemented so I went with this version that keeps the reducers simpler.

[fhelwanger]

I don't get it, I was thinking about something like this:

export function createNode() {
  return {
    type: CREATE_NODE,
    nodeId: generateId()
  }
}

export function addChild(nodeId) {
  return {
    type: ADD_CHILD,
    nodeId,
    childId: generateId()
  }
}

let nextId = 0
function generateId() {
  return `new_${nextId++}`
}

How does it make the reducers more complex?

Sorry if I'm missing something 😄

[gaearon]

Please try and submit a PR if it works. It's hard to discuss hypothetical code. ;-)

[fhelwanger]

Nice, I'll do it :)

[fhelwanger]

Oh, I got it now 😓

When you call const childId = createNode().nodeId you're actually dispatching the action because of mapDispatchToProps.

When reading the code, I assumed it was being called only to get the new id.

You were right, it's hard to discuss hypothetical code :-)

Sorry for asking without testing first, and thank you so much for all the good work you've been doing 😄

[gaearon]

No problem 😉 If you do find a way to make this code simpler let us know!