This isn't used anywhere else and doesn't seem necessary? Seems like an unnecesary indirection instead of just placing an empty array (eg [])
Depends on D4195
Details
Details
Didn't test
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| lib/reducers/thread-reducer.js | ||
|---|---|---|
| 156 ↗ | (On Diff #13311) | It seems unlikely that it was added without a good reason. In this case, I guess there might be a place where we compare newInconsistencies with oldInconsistencies using referential equality and this change might break that: [] !== [], but emptyArray === emptyArray. Please investigate it. |
Comment Actions
Re-requesting review after responding to comment.
| lib/reducers/thread-reducer.js | ||
|---|---|---|
| 156 ↗ | (On Diff #13311) | That's a good point about keeping the reference the same for purposes of shallow equality. However, we only use the findInconsistencies function for the processServerRequests action type. For all of the other action types we're returning a new empty array when there are no inconsistencies. Further, at the reduceThreadInfos callsite in lib/reducers/master-reducer we almost immediately "spread" the value of newThreadInconsistencies "into" newInconsistencies. Nowhere along the way do we do a shallow comparison of the inconsistencies array, so I think it's safe for it to be removed? |
| lib/reducers/thread-reducer.js | ||
|---|---|---|
| 156 ↗ | (On Diff #13311) | If we don't compare shallowly in any place, it should be ok |