mattkrick / redux-optimistic-ui

a reducer enhancer to enable type-agnostic optimistic updates
MIT License
693 stars 36 forks source link

Bug when attempting to commit / revert with invalid id #32

Closed marcins closed 7 years ago

marcins commented 7 years ago

I was working on improving the coverage reporting and noticed the cases dealing with an invalid transaction id aren't covered so was trying to write some tests for this, however I don't believe that the current behaviour when attempting to commit or revert with an invalid id is correct, because the invalid action gets pushed to the history and then "found".

Here's the scenario, using the INC/DEC reducer from tests:

So the console error never gets triggered, and an additional, potentially invalid, action exists in history.

I haven't gone through the logic for a revert fully, but I believe a similar case exists - although in this case the delete on line 85 will cause the invalid item in the history to be deleted - but no console.error either since the invalid id will actually be found in the history.

My proposal: move the invalid id check against the history to Line 125 - throw an error rather than just console.error to stop execution.

Is my assessment correct? Are you happy with the solution? If so I'm happy to implement it.

marcins commented 7 years ago

I've gone ahead an pushed my proposed fix anyway, as #33 😄