javascript - React - Triggering click event on table row

Question

I am having trouble to invoke click event from a table row in React. Below is my code. I have a separate function to populate the rows alone, but it seems i am messing up the binding information to that.

import React, {Component, PropTypes} from 'react';
import Header from './Header';

export default class SongData extends Component {
 constructor(props, context) {
    super(props, context);
 }

isEmpty(obj) {
    return Object.keys(obj).length === 0;
}

fetchDetails(song) {
    console.log(song);
}

renderResultRows(data) {
    var self = this;
    return data.map(function(song) {
        return (
            <tr onClick={self.fetchDetails(song)}>
                <td data-title="Song">{song.S_SONG}</td>
                <td data-title="Movie">{song.S_MOVIE}</td>
                <td data-title="Year">{song.S_YEAR}</td>
            </tr>
        );
    }.bind(this));
}

render() {
    return (
        <div id="no-more-tables">
            <table className="table table-hover table-bordered table-striped">
                <thead>
                    <tr>
                        <th>Song</th>
                        <th>Movie</th>
                        <th>Year</th>
                    </tr>
                </thead>
                <tbody>
                    {!this.isEmpty(this.props.searchResult)
                        ? this.renderResultRows(this.props.searchResult)
                        : ''}
                </tbody>
            </table>
        </div>
    );
}

}

Answer 1

Firstly, you are passing an evaluated function call to your onClick property.

See the following code in isolation:

 function foo(name) { return 'hello ' + name; }

 foo('bob');

You expect the output of foo('bob') to be "hello bob".

It's the exact same if I passed it into on onClick prop. For example:

 <button onClick={foo('bob')}

In this case I will just be passing a string to the onClick prop for the button. The onClick (and other event props) expect a function (or ref) to be provided. I'll give an example of this a bit further down.

Secondly I see you have the right intention in trying to maintain the correct scope for your functions using a combination of const self = this and bind. There is an easier way to do so using anonymous functions from ES6/2015, which always maintain the scope of where they were declared.

I could then update your code to the following:

renderResultRows(data) {
    return data.map((song) => {  // anon func maintains scope!
        // Pass in a function to our onClick, and make it anon
        // to maintain scope.  The function body can be anything
        // which will be executed on click only.  Our song value
        // is maintained via a closure so it works.
        return (
            <tr onClick={() => this.fetchDetails(song)}>
                <td data-title="Song">{song.S_SONG}</td>
                <td data-title="Movie">{song.S_MOVIE}</td>
                <td data-title="Year">{song.S_YEAR}</td>
            </tr>
        );
    });  // no need to bind with anon function
}

But this is still not optimal. When using the anonymous syntax like this (e.g. <foo onClick={() => { console.log('clicked') }) we are creating a new function on every render. This could be an issue if you are using pure components (i.e. components that should only re-render when new prop instances are provided - this check being performed via a shallow compare). Always try to create your functions early, e.g. in the constructor or via class properties if you are using babel stage 1, that way you always pass in the same reference.

For your example however you require a value to be "passed into" each of the onClick handlers. For this you could use the following approach:

fetchSongDetails = () => {
  const song = e.target.getAttribute('data-item');
  console.log('We need to get the details for ', song);
}

renderResultRows(data) {
    return data.map((song, index) => {  // anon func maintains scope!
        // Pass in a function to our onClick, and make it anon
        // to maintain scope.  The function body can be anything
        // which will be executed on click only.  Our song value
        // is maintained via a closure so it works.
        return (
            <tr key={index} data-item={song} onClick={this.fetchSongDetails}>
                <td data-title="Song">{song.S_SONG}</td>
                <td data-title="Movie">{song.S_MOVIE}</td>
                <td data-title="Year">{song.S_YEAR}</td>
            </tr>
        );
    });  // no need to bind with anon function
}

That's a much better approach to follow. Also note that I added a key prop to each row being generated. This is another react best practice as react will use the unique key identifiers in it's diff'ing algorithm.

---

I highly recommend the following reading:

• Scope and Closures: https://github.com/getify/You-Dont-Know-JS/tree/master/scope%20%26%20closures
• React & ES6/2015: http://egorsmirnov.me/2015/08/16/react-and-es6-part3.html
• React & dynamic children: https://facebook.github.io/react/docs/multiple-components.html#dynamic-children