Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: store entire history to improve undo/redo robustness #154

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

chrisvxd
Copy link
Member

Please approve #153 first

Some scenarios were tripping up the undo/redo history because of the delta history that used deep-diff.

Specifically, this occurred when implementing array field reordering in #143, which caused the undo/redo to corrupt the ordering. Presumably a race condition, as could not easily reproduce during unit tests (which will be added in #143).

It turns out that we were storing the entire object in the forward history (rather than just the diff) anyway, so I decided to strip out deep-diff and store the entire object for now. In future, we should revisit delta history.

FYI @Yuddomack

@chrisvxd chrisvxd requested a review from monospaced October 17, 2023 10:48
@vercel
Copy link

vercel bot commented Oct 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
puck-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 17, 2023 0:01am

@chrisvxd chrisvxd mentioned this pull request Oct 17, 2023
Base automatically changed from record-app-state-in-reducer to main October 17, 2023 11:58
Some scenarios were tripping up undo/redo history when implementing array field reordering. Presumably a race condition, as could not reproduce during unit tests. It turns out that we were storing the entire object for the forward history, anyway, so this needs revisiting.
@chrisvxd chrisvxd force-pushed the store-entire-history branch from ff726a2 to 1211315 Compare October 17, 2023 11:59
@chrisvxd chrisvxd merged commit a7d9a28 into main Oct 17, 2023
2 checks passed
@chrisvxd chrisvxd deleted the store-entire-history branch October 17, 2023 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants