search

vigetlabs / microcosm

Flux with actions at center stage. Write optimistic updates, cancel requests, and track changes with ease.
http://code.viget.com/microcosm/
MIT License
487 stars 22 forks source link

Toggle (redo) is currently not working in versions >= 12.10 #525

Closed damianhouse closed 5 years ago

damianhouse commented 5 years ago

We ran into a scenario at my job where we upgraded microcosm from 12.6.1 to 12.13.2 and our redo no longer functioned properly. After some research we realized that the bug is within microcosm and was introduced in version 12.10.0. The issue lies within the updateSnapshot function when an action has been toggled (undo) and then you toggle again (redo). const action1 = repo.push(myaction1, mypayload1) const action2 = repo.push(myaction2, mypayload2) repo.history.toggle(action2) //expect: to see only payload after action1 //actually: works correctly repo.history.toggle(action2) //expect: to see payload after action2 //actually: see payload after action1 We made a very basic app with microcosm to help us diagnose the issue in this repo https://github.com/damianhouse/microcosmredoissue.

In domain-engine.dispatch method if the start point, payload, and status are the same between the action being toggled and the snapshot then the cached snapshot is used. The issue lies in microcosm.updateSnapshot method where the cache is overwritten when an action is disabled and nothing else is changed to trigger a recalculation of the cache when the action is re-enabled. We found that we could fix this by altering the snapshot status when toggling an action which would trigger a recalculation, however this is inefficient because the cache should be valid if the input and the payload are the same. In other words, the snapshot should be ignored when disabling an action rather than using the overwritten cache.

I hope this information was helpful, thank you for your work on microcosm!

nhunzaker commented 5 years ago

Hey Damian!

Thank you for such an excellent write up! I'm looking into this today and I'll touch base before the day wraps up.

damianhouse commented 5 years ago

Just so you know, we refactored our undo/redo code to use checkout instead of toggle so we probably won't be able to verify your fix in our app.

nhunzaker commented 5 years ago

No worries either way, this is a bug and should be fixed!

I've sent a fix out here: https://github.com/vigetlabs/microcosm/pull/526

Thanks again for your excellent analysis of the issue. That was precisely the problem!

nhunzaker commented 5 years ago

I sent out a release candidate for a version with this fix: 

npm install microcosm@12.15.0-rc.0

It's been a while since we had a release, and I updated some dependencies. I'll verify the build on Monday and release to stable.

Thanks again!

damianhouse commented 5 years ago

Awesome, thank you and happy holidays!

nhunzaker commented 5 years ago

You too! Sorry I never followed up here, I've released microcosm@12.15.0.