UndoMiddleware saves patches from non-targeted store.

[zuhorer]

Bug report

I've checked documentation and searched for existing issues
I've made sure my project is based on the latest MST version
Fork this code sandbox or another minimal reproduction.
Sandbox link or minimal reproduction code
https://codesandbox.io/s/mobx-state-tree-todolist-forked-kxzoqw
Above you can see my minimal reproduction of graphics editor. We have a BoxStore with an UndoMiddleware("history" property) where we save our graphics elements and Focus store(for UI) where we save array of references to these graphics elements.
Then we do such steps:

Add the new Box to the BoxStore.
Focus Box(add reference to this box into Focus store).
Remove the Box from the BoxStore(at this step, the reference to the Box invalidates).
Undo the previous action(recover the removed Box).
And after last step, not only Box is recovered, but also the reference to this box in the Focus store is recovered too.

From the UndoMiddleware Docs I understand that only changes in targetStore (in out case it is BoxStore) should be tracked. But as I see if the action invokes from the targetStore and this action change the state of secondary store(in our case it is Focus), all the patches (includes the changing of the Focus store) are writing to the UndoMiddleware.
image

Describe the expected behavior
Only patches from targetStore should be written to the UndoMiddleware.

Describe the observed behavior
All the patches are written to the UndoMiddleware.

[songzhenqi]

We can fix this by filter in recordPatches.

code ( in undo-manager.ts):

onStart(call) {
                const recorder = recordPatches(
                    call.tree,
                    (_patch, _inversePatch, actionContext) => {
                        if (recordingDisabled) {
                            return false
                        }
                        
                        // only record patches generated by targetStore
                        if(!_patch.path.startsWith(getPath(targetStore))) {
                            return false
                        }

                        // only record patches that were generated by this action or children of this action
                        return (
                            !!actionContext && isActionContextThisOrChildOf(actionContext, call.id)
                        )
                    }
                )
                recorder.resume()
                call.env = {
                    recorder
                }
            },

BTW ,I've trimmed down the code in the example to minimize the cost of reproducing it.

import UndoManager from "./undo/undo";

import { types, destroy, getSnapshot } from "mobx-state-tree";

let UID = 1;
const Box = types
    .model("Box", {
        id: types.optional(types.identifier, ''),
    })

const BoxStore = types
    .model("BoxStore", {
        boxes: types.array(Box)
    })
    .actions((self) => ({
        afterCreate() {
            setUndoManager(self);
        },
        addRandomBox() {
            const randomBox = Box.create({ id: `box-${UID++}`, });
            self.boxes.push(randomBox);
            return randomBox;
        },
        remove(box: any) {
            destroy(box);
        }
    }))

const Focus = types
    .model("Focus", {
        focusedBoxes: types.array(types.safeReference(Box))
    })
    .actions(self => ({
        toggleBoxFocus(box: any) {
            if (self.focusedBoxes.includes(box)) {
                self.focusedBoxes.remove(box);
            } else {
                self.focusedBoxes.push(box);
            }
        }
    }))

const RootStore = types
    .model("RootStore", {
        boxStore: BoxStore,
        focus: Focus,
    })

const store = RootStore.create({
    boxStore: {
        boxes: []
    },
    focus: {
        focusedBoxes: []
    }
});

let manager: any = null
const setUndoManager = (targetStore: any) => {
    manager = UndoManager.create({}, { targetStore })
};

const box = store.boxStore.addRandomBox();
console.log('history 1', JSON.stringify(getSnapshot(manager)))

store.focus.toggleBoxFocus(box);
console.log('history 2', JSON.stringify(getSnapshot(manager)))

store.boxStore.remove(box);
console.log("history 3: ", JSON.stringify(getSnapshot(manager))) // history3 should not contains patches from store.focus

@coolsoftwaretyler Do I need to submit a PR？

[coolsoftwaretyler]

@songzhenqi, yes if you have time I'd love a PR for this. If not, I'll get around to it but probably not for a while.

Let me know if you plan on writing one!

[songzhenqi]

I'd like do it tomorrow.

I actually did some improvement work on undomanager.
I'll start a new discussion for communication

[coolsoftwaretyler]

Sounds great! Thanks, I really appreciate it