feat: add store-level getUndoPath for handle-driven navigation-aware undo#47
Merged
Merged
Conversation
…undo
Handle writes (update/add/remove) push undo actions through makeOnPushUndo,
which never set UndoAction.path — so onNavigate was dead for every undo except
a manual undoManager.push({ path }). Add a store-level getUndoPath callback,
stamped onto every handle-pushed action, so the app supplies the router path
at write time and navigation-aware undo works for ordinary writes.
Exposed on FirestateConfig and FirestateProviderProps with the same
runtime-replace plumbing as onNavigate/onError.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Handle writes (
update/add/remove) push undo actions throughmakeOnPushUndo, which never setUndoAction.path. Soaction.pathwas alwaysundefinedfor handle-driven undo andonNavigatenever fired — only a manualundoManager.push({ path })could trigger navigation-aware undo, which defeats the point.Fix
Add a store-level
getUndoPath?: () => string | undefinedcallback onFirestateConfigandFirestateProviderProps;makeOnPushUndonow stampspath: store.getUndoPath()onto every handle-pushed action, so the app supplies the router path at write time andonNavigateworks for ordinary writes. It uses the same runtime-replace plumbing (setGetUndoPath+useEffect) asonNavigate/onError, and existing group-merge semantics are untouched (merged groups keep the newest action's path). Covered by store-level unit tests and an end-to-end integration test through the realmakeOnPushUndopath, plus README docs; full suite passes (258 tests) and typecheck/build are clean.Note
Add
getUndoPathcallback to stamp router path onto handle-driven undo actionsgetUndoPath?: () => string | undefinedtoFirestateConfig,FirestateStore, andFirestateProvider, allowing callers to supply a function that returns the current router path.store.getUndoPath()is stamped onto the action so thatonNavigateis invoked on undo/redo.FirestateStoreexposessetGetUndoPathto replace the resolver at runtime without recreating the store;FirestateProvidercalls this in an effect when the prop changes.📊 Macroscope summarized 2261e2a. 5 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.