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: