Introduction
This is just a little article to share a problem I encountered. I was tasked with building a CRM and transactions had to be filtered by date. Nothing fancy about all this. You check if the transaction's date is between two other dates the user gives you ( a start date and an end one ).
The code
Well, here is how the code looked before I managed to solve it. I was using React with this code. I used the popular javascript library moment.js to work with dates.
renderTransactions(){
return this.props.transactions.filter( transaction => {
const start = moment(this.state.startDate)
const end = moment(this.state.endDate)
const tDate = moment(transaction.date)
return tDate.isBetween(start, end) // Compare dates here
})
.map( ( transaction, i ) => {
transaction.date = moment(transaction.date).format('DD/MM/YYYY')
return (
<tr key={i}>
<td>{transaction.amount}</td>
<td>{transaction.date}</td>
<td>{transaction.client}</td>
</tr>
})
}
// etc.. etc...
Ok, can you see where the problem is? The first time the component renders, everything is fine, the dates are properly filtered. But, as soon as I modify the dates, nothing works anymore...
The problem was here:
transaction.date = moment(transaction.date).format('DD/MM/YYYY')
See how stupid that is? I mutate the state of my array with this line. On the next render, the date field is not longer a Date object that the moment library can work with, but a String...
Solving it, and restoring my sanity...
Take one:
Don't use this.props directly. If I used a reference to this.props.transactions in my function, the problem wouldn't have happened. Every time the function would run, a fresh copy of this.props.transactions would be used. Even if I kept the problematic line with the direct mutation, it would work.
Take two:
Immutability. Don't mutate the state of your application directly. This line would have solved everything:
const date = moment(transaction.date).format('DD/MM/YYYY')
Done... Problem solved.
Conclusion
I'm just writing this to make sure I don't make the same stupid mistake again. I spent way to much time on this... I'm tired and I hate programming.
Top comments (2)
Why do you use an extra variable to hold the date string? You can do the conversion at the place, where it's needed, like this:
<td>{moment(transaction.date).format('DD/MM/YYYY')}</td>?
Using an extra variable makes sense, when the computation of the result is performance intense or being used at several places.
Alternatively you could have used object destructuring assignment in the arguments, to avoid mutating the original object:
This is not only shorter to write, it also gives you the opportunity to default missing values (e.g.
({someValue = "example default value"}) => ...) and a convenient way to rename keys, very useful to keep .map() an understandable one liner: e.g.({objectKey: newNameForObjectKey, ...rest}) => ({...rest, newNameForObjectKey}).The object rest spread (
...rest) gives you an object, that contains every key, you did not specify in the destructuring.I hope this will help to avoid problems --like the one you described-- in the future 😊
Yeah, putting the moment() inside the td element would have been simpler indeed :D When you're stuck on a problem, it seems like certain things are invisible to you :)
I actually never thought about using destructuring with .map(), this could be really useful!