undo/redo middleware support for drafts

[mindreframer]

Reproducible test case here:

• https://github.com/mindreframer/mobx-keystone-draft-issue

Code:

import { toJS } from "mobx";
import { Draft, draft, model, Model, modelAction, prop } from "mobx-keystone";

/**
 * In this example we use a mobx - keystone instance for Settings
 * that is reused in the form as plain non-observable field
 */

@model("myApp/Tag")
export class Tag extends Model({ label: prop<string>("").withSetter() }) {}

/**
 * actual settings data
 */
@model("myApp/Settings")
export class Settings extends Model({
  name: prop<string>().withSetter(),
  tags: prop<Tag[]>(() => []),
}) {}

/**
 * Settings form, contains functions to manipulate settings / run validations
 * - we want to support undo/redo for it + editing on a detached node (drafts)
 * - that's why draft is a `prop`, and not just plain object field
 */
@model("myApp/SettingsForm")
export class SettingsForm extends Model({
  settingsDraft: prop<Draft<Settings> | undefined>(),
}) {
  @modelAction setSettings(val: Settings) {
    this.settingsDraft = draft<Settings>(val);
  }
  @modelAction deleteTag(tags: Tag[], index: number) {
    tags.splice(index, 1);
  }
}

/**
 * Top-level node
 */
@model("myApp/Root")
export class Root extends Model({
  settings: prop<Settings | undefined>(),
  settingsForm: prop<SettingsForm | undefined>(),
}) {
  onInit() {
    if (this.settings) {
      this.setSettings(this.settings);
    }
  }
  @modelAction setSettings(val: Settings) {
    this.settings = val;
    this.settingsForm = new SettingsForm({});
    this.settingsForm.setSettings(this.settings);
  }
}

Not sure what I'm doing wrong here...

[xaviergonz]

this.settingsDraft = draft<Settings>(val);

you are trying to assign as a child node something that is not a node / cannot be converted to a node. draft is not really a model or anything, but really an api that works over the data you pass inside.

maybe what you intended is to make settingsDraft a plain variable (outside of mobx-keystone?

@model("myApp/SettingsForm")
export class SettingsForm extends Model({
  settings: prop<Settings | undefined>(),
}) {
  @observable.ref
  settingsDraft?: Draft<Settings>;

  @modelAction setSettings(val: Settings) {
    this.settings = val;
    this.settingsDraft = draft<Settings>(val);
  }
  @modelAction deleteTag(tags: Tag[], index: number) {
    tags.splice(index, 1);
  }
}

Although I'd advise you to create the draft in the form component rather than in the model and then do a draft.commit() once you press the "save" button

[xaviergonz]

@sisp might be better at explaining how draft works than I am, since it was his idea :)

[xaviergonz]

Looking at it all again it looks to me like SettingsForm in its totality should not even be part of the mobx-keystone state model, but of the from component state.

[mindreframer]

@xaviergonz OK, understood! So drafts do not belong to models. I'll try your suggestion and see if it works better!

Thanks for the super-quick response!

[xaviergonz]

no problem 👌

[mindreframer]

I wanted to have a place to arrange functions for data manipulation and I wanted to keep it outside of the settings data model. It can be a big data structure and my desire was to display / modify / validate parts of settings model in form instances, that hold on to a subtree from the full settings model. To be able to use @modelAction decorator I have decided to implement the form classes as mobx-keystone state model. (I tried to use plain mobx classes, but this led to some issues)

So, basically my requirements are:

• support undo/redo middleware
• support drafts (so that the currently edited subtree is not propagating changes to the complete settings model)

I'll provide a more complete example

[mindreframer]

@xaviergonz I have updated the example to following code:

• https://github.com/mindreframer/mobx-keystone-draft-issue/blob/main/src/use-with-indo.ts

  
  import { observable } from "mobx";
  import { Draft, draft, model, Model, modelAction, prop } from "mobx-keystone";

@model("myApp/Tag") export class Tag extends Model({ label: prop("").withSetter() }) {} @model("myApp/Settings") export class Settings extends Model({ name: prop("").withSetter(), tags: prop<Tag[]>(() => []), }) {} @model("myApp/Root") export class Root extends Model({ settings: prop<Settings | undefined>(), }) { @observable.ref settingsDraft: Draft | undefined; onInit() { if (this.settings) { this.setSettings(this.settings); } } @modelAction setSettings(val: Settings) { this.settings = val; this.settingsDraft = draft(this.settings); } @modelAction commit() { this.settingsDraft?.commit(); }

@modelAction deleteTag(index: number) { this.settingsDraft?.data.tags.splice(index, 1); } }

```ts
    const root = new Root({});
    root.setSettings(new Settings({}));
    const settings = new Settings({
      tags: [new Tag({ label: "tag1" }), new Tag({ label: "tag2" })],
    });
    root.setSettings(settings);
    // wrap just the data part from draft with the undoMiddlware
    const undoManagerSettingsDraft = undoMiddleware(root.settingsDraft!.data);
    root.deleteTag(1);
    logAll(root.settingsDraft, "DRAFT BEFORE COMMIT");
    logAll(undoManagerSettingsDraft.store.undoEvents, "UNDO EVENTS:");
    root.commit();
    logAll(root.settingsDraft, "DRAFT AFTER COMMIT");

The undoManager is not recording any changes to the draft.

Is it possible to use the undo middleware in combination with drafts at all?

[mindreframer]

undoMiddleware records only changes made directly by actions on the settings model. If we make changes through the root model, they are not recorded .

import { observable } from "mobx";
import { Draft, draft, model, Model, modelAction, prop } from "mobx-keystone";

@model("myApp/Tag")
export class Tag extends Model({ label: prop<string>("").withSetter() }) {}
@model("myApp/Settings")
export class Settings extends Model({
  name: prop<string>("").withSetter(),
  tags: prop<Tag[]>(() => []),
}) {
  @modelAction deleteTag(index: number) {
    this.tags.splice(index, 1);
  }
}
@model("myApp/Root")
export class Root extends Model({
  settings: prop<Settings | undefined>(),
}) {
  @observable.ref settingsDraft: Draft<Settings> | undefined;
  onInit() {
    if (this.settings) {
      this.setSettings(this.settings);
    }
  }
  @modelAction setSettings(val: Settings) {
    this.settings = val;
    this.settingsDraft = draft(this.settings);
  }
  @modelAction commit() {
    this.settingsDraft?.commit();
  }

  @modelAction deleteTag(index: number) {
    this.settingsDraft?.data.tags.splice(index, 1);
  }
}

describe("Root", () => {
  it("should support drafts wrapped in undo/redo middleware ", () => {
    const root = new Root({});
    root.setSettings(new Settings({}));
    const settings = new Settings({
      tags: [new Tag({ label: "tag1" }), new Tag({ label: "tag2" })],
    });
    root.setSettings(settings);
    const undoManagerSettingsDraft = undoMiddleware(root.settingsDraft!.data);
    const draftModel = root.settingsDraft!;

    root.deleteTag(0); // this is not recorded
    logAll(root.settingsDraft, "DRAFT after modification by root");

    draftModel.data.deleteTag(0); // this is recorded
    logAll(root.settingsDraft, "DRAFT after direct modification");

    root.settingsDraft?.data.setName("HELLO");
    logAll(undoManagerSettingsDraft.store.undoEvents, "UNDO EVENTS:");
    root.commit();
    logAll(root.settingsDraft, "DRAFT AFTER COMMIT");
  });
});

I guess this is something that one has to take into consideration if undo/redo if of importance.

[xaviergonz]

I don't think I ever tried, but I'd try attaching an undo middleware to the draft(settings).data. That object the modified clone of the settings (in a separate tree) and gets merged into the main tree once the commit function is called.

[xaviergonz]

Ah, I see you just did that. In order to support undo/redo to both the root store and the draft I guess you'd need to attach an undo middleware to each. Your issue is that you want to combine both undo/redo event sources into a single queue I guess?

[mindreframer]

Yes, I guess I wanted to be smug and have it all without effort 😄 Tracking just the draft, yet keep logic for modification in other classes. Had to change my strategy, now all actions are on the Settings class.

This is a closer representation of the required model:

• https://github.com/mindreframer/mobx-keystone-draft-issue/blob/d13192b161ec6a6882d0f7d2087514728a855f1d/src/use-with-indo.ts

import { observable } from "mobx";
import {
  Draft,
  draft,
  findParentPath,
  model,
  Model,
  modelAction,
  prop,
  resolvePath,
} from "mobx-keystone";

@model("myApp/Tag")
export class Tag extends Model({ label: prop<string>("").withSetter() }) {}
@model("myApp/Form1")
export class Form1 extends Model({
  tags: prop<Tag[]>(() => []).withSetter(),
  name: prop<string>("").withSetter(),
}) {}

@model("myApp/Form2")
export class Form2 extends Model({
  tags: prop<Tag[]>(() => []).withSetter(),
  name: prop<string>("").withSetter(),
}) {}

export namespace SettingsUtils {
  /**
   * pathFromChild
   * @param child
   * @returns list of path segments for the given child under Settings
   */
  export const pathFromChild = (child: any) => {
    const res = findParentPath(
      child,
      (parent) => parent instanceof Settings,
      5 // how deep?
    );
    return res?.path;
  };

  /**
   *
   * @param path list of path segments
   * @param instance settings instance to resolve the path on
   * @returns found child instance / undefined
   */
  export const childFromPath = (path: Path | undefined, instance: Settings) => {
    if (!path) {
      return;
    }
    return resolvePath(instance, path);
  };
}

@model("myApp/Settings")
export class Settings extends Model({
  name: prop<string>("").withSetter(),
  form1List: prop<Form1[]>(() => []).withSetter(),
  form2List: prop<Form2[]>(() => []).withSetter(),
  tags: prop<Tag[]>(() => []),
}) {
  @modelAction arrayRemoveAt(array: any[], index: number) {
    array.splice(index, 1);
  }

  @modelAction arrayPush(array: any[], item: any) {
    array.push(item);
  }

  @modelAction addForm1() {
    this.form1List.push(new Form1({}));
  }

  @modelAction addForm2() {
    this.form2List.push(new Form2({}));
  }
}

type Path = ReadonlyArray<string | number>;

@model("myApp/Root")
export class Root extends Model({
  settings: prop<Settings | undefined>(),
  selectionPath: prop<Path | undefined>(),
}) {
  @observable.ref settingsDraft: Draft<Settings> | undefined;
  @observable.ref selectedChild: any;
  onInit() {
    if (this.settings) {
      this.setSettings(this.settings);
    }
  }
  @modelAction setSettings(val: Settings) {
    this.settings = val;
    this.settingsDraft = draft(this.settings);
    this.setChildFromSelectionPath();
  }
  @modelAction commit() {
    this.settingsDraft?.commit();
  }

  @modelAction setSelectionPath(child: any) {
    this.selectedChild = child;
    this.selectionPath = SettingsUtils.pathFromChild(child);
  }

  @modelAction setChildFromSelectionPath() {
    if (!this.selectionPath) {
      return;
    }
    const res = SettingsUtils.childFromPath(
      this.selectionPath,
      this.settingsDraft?.data!
    );
    if (res?.resolved) {
      this.selectedChild = res.value;
    } else {
      this.selectedChild = undefined;
    }
  }
}

This seems to work OK. I also need to hold on to the path of the subtree currently in edit mode, because when I update the model with the response from the backend after saving the objects in-memory become stale and I need to reflect the server update for a particular subtree on the screen. By keeping the path I can resolve a fresh subtree for this path and swap the subtree model with the updated one.

Not sure if this is somewhat over-engineered, I could not figure out a simpler or more straightforward solution.

[mindreframer]

@xaviergonz I'm going to close this issue, and I hope this discussion might be useful for somebody else hitting similar issues. Thanks for quick response!

Another side-note: those array helper functions that I have used (arrayPush and arrayRemove) can be replaced with the build-in standardActions - https://mobx-keystone.js.org/standardActions. It took me some time to figure that out 😅