m0onspell
m0onspell

Reputation: 545

React/Redux: separating presentational and container components dilemma.

Here's a simplified representation of what I have:

container.js:

import Presentation from './Presentation';
import { connect } from 'react-redux';

export default connect(
    ({ someState}) => ({ ...someState }),
    (dispatch) => {
        onClick(e) => ({})
    }
)(Presentation);

pesentation.js:

export default ({ items, onClick}) => (
    <table>
        <tbody>
            {items.map((item, index) => (
                <tr>
                    <td onClick={onClick}>{item.a}</td>
                    <td>
                        <div onClick={onClick}>{item.b}</div>
                    </td>
                </tr>
            ))}
        </tbody>
    </table>
);

Let's say, I want to have access to index variable of each row in handler. First solution that comes into mind is to use a closure for capturing index:

container.js:

onClick(e, index) => ({});

presentation.js:

<td onClick={e => onClick(e, index)}>{item.a}</td>
<td>
    <div onClick={e => onClick(e, index)}>{item.b}</div>
</td>

It works, but it looks quite dirty, so I decided to find another solution if possible. The next thing is to use data-index attribute on each element:

container.js:

onClick({ target: { dataset: { index } } }) => ({});

presentation.js:

<td data-index={index} onClick={onClick}>{item.a}</td>
<td>
    <div data-index={index} onClick={onClick}>{item.b}</div>
</td>

Maybe it's better, but the need to use data-attribute with same value on multiple elements looks redundant. What if I have 20 elements? Ok, no problem! Let's just move it to parent container:

container.js:

onClick(e) => {
    const {index} = e.target.closest('tr').dataset;
};

presentation.js:

<tr data-index={index}>
    <td onClick={onClick}><td>
    <td>
        <div onClick={onClick}>{item.b}</div>
    </td>
</tr>

Ok, the next problem is solved. But wait. This way, container heavily relies on presentational component layout. It becomes coupled to view by introducing reference to tr tag. What if at some point tag will be changed to, for example, div? The final solution that I come up with is to add a CSS class for container tag element, which is passed from container component to presentation:

container.js:

import Presentation from './Presentation';
import { connect } from 'react-redux';

const ITEM_CONTAINER_CLASS = 'item-cotainer';

export default connect(
    ({ someState }) => ({ ...someState, ITEM_CONTAINER_CLASS }),
    (dispatch) => {
        onClick(e) => {
            const {index} = e.target.closest(`.${ITEM_CONTAINER_CLASS }`).dataset;
        }
    }
)(Presentation);

pesentation.js:

export default ({ items, onClick, ITEM_CONTAINER_CLASS  }) => (
    <table>
        <tbody>
            {items.map((item, index) => (
                <tr className={ITEM_CONTAINER_CLASS}>
                    <td onClick={onClick}>{item.a}</td>
                    <td>
                        <div onClick={onClick}>{item.b}</div>
                    </td>
                </tr>
            ))}
        </tbody>
    </table>
);

But still, it has one small disadvantage: it requires a DOM manipulation to get index.

Thanks for reading (if someone). I wonder, if there are another options?

Upvotes: 2

Views: 407

Answers (2)

m0onspell
m0onspell

Reputation: 545

See also an accepted answer. Here's the solution:

container.js

import Presentation from './presentation';
import { connect } from 'react-redux';

export default connect(
    ({ someState }) => ({ ...someState }),
    (dispatch) => {
        onClick(index) => {}
    }
)(Presentation);

presentation.js

import Item from './item.js';

export default ({ items, onClick }) => (
    <table>
        <tbody>
            {items.map((item, index) => <Item index={index} item={item} onClick={onClick} />)}
        </tbody>
    </table>
);

item.js

export default ({ index, item, onClick }) => {
    const onItemClick = () => onClick(index);
    return (
        <tr>
            <td onClick={onItemClick}>{item.a}</td>
            <td>
                <div onClick={onItemClick}>{item.b}</div>
            </td>
        </tr>
    )
};

Upvotes: 0

CharlieBrown
CharlieBrown

Reputation: 4163

This is classical React/Redux dilemma. My answer would be to extract your "item view" to a component and add an onItemClick prop to it. Of course your item view should receive the item to display, and will be able to send it as a parameter to your container.

The e parameter belongs to your presentation, not to your action creator which is business logic. And storing data in the DOM (like using data- attributes is not needed with React at all).

Something like this (I tend to start from bottom to top, from simple to complex):

//Item.js
export default class Item extends Component {
   constructor(props){
     super(props)
     this.handleClick = this.handleClick.bind(this)
   }
   handleClick(e){
     e.preventDefault() //this e stays here
     this.props.onItemClick(this.props.item)
   }
   render(){
      const { item } = this.props
      return (
        <tr className={ITEM_CONTAINER_CLASS}>
          <td onClick={this.handleClick}>{item.a}</td>
          <td onClick={this.handleClick}>{item.b}</td>
        </tr>
      )
   }
}

// ItemList (Presentation)
export default ItemList = ({ items, onClick}) => (
  <table>
    <tbody>
      {
        items.map((item, index) => 
          <Item item={item} onItemClick={ onClick } />
        )
      }
    </tbody>
  </table>
)

// Container.js - just change your mapDispatchToProps to accept the full item, or whatever you're interested in (perhaps only the id)
import ItemList from './ItemList';
import { connect } from 'react-redux';

export default connect(
    ({ someState}) => ({ ...someState }),
    {
       onClick(item){  
         return yourActionCreatorWith(item.id)
       }
    }
)(Presentation);

That way you have everything you need right when you need to dispatch. In addition, creating your "item view" as a separate component will allow you to test it in isolation and even reuse it if needed.

It may seem much more work but trust me, it pays off.

Upvotes: 4

Related Questions