mobxjs / mobx-state-tree

Full-featured reactive state management without the boilerplate
https://mobx-state-tree.js.org/
MIT License
6.93k stars 641 forks source link

Support JSON Patch move operation #1260

Open johnrees opened 5 years ago

johnrees commented 5 years ago

Feature request

Is your feature request related to a problem? Please describe.

The application I am currently working on uses a very large tree structure. I often need to move nodes around and patches currently delete the node (and all of its descendants) then recreate it in the new location. It is a collaborative editor and these patches are broadcast via websockets and it seems wasteful to need to recreate large sub-trees each time a node is moved.

Describe the solution you'd like

I would like to use the move operation instead http://jsonpatch.com/#move

Describe alternatives you've considered

I've hacked together a workaround/example here https://codesandbox.io/s/18xjlrnz14

Everything is in the console, basically it collects patches in an array then when the onSnapshot is triggered it looks through the array to see if there are any remove+add operations with the same ID, and replaces them with a single move operation.

In the example this tree is created...

- grampa simpson
  - herb
  - homer
    - bart
    - maggie
    - lisa

then maggie and homer are moved and (moved) is appended to their names

- grampa simpson
  - homer (moved)
    - bart
    - lisa
    - maggie (moved)
  - herb

using regular MST the patch/inverse operations for moving involve removing and adding the node. This is to move homer and append (moved) to his name.

{
  "processed": [
    {
      "patch": {
        "op": "remove",
        "path": "/children/1"
      },
      "inverse": {
        "op": "add",
        "path": "/children/1",
        "value": {
          "id": "homer",
          "name": "homer",
          "children": [
            {
              "id": "bart",
              "name": "bart",
              "children": []
            },
            {
              "id": "lisa",
              "name": "lisa",
              "children": []
            },
            {
              "id": "maggie",
              "name": "maggie (moved)",
              "children": []
            }
          ]
        }
      }
    },
    {
      "patch": {
        "op": "add",
        "path": "/children/0",
        "value": {
          "id": "homer",
          "name": "homer (moved)",
          "children": [
            {
              "id": "bart",
              "name": "bart",
              "children": []
            },
            {
              "id": "lisa",
              "name": "lisa",
              "children": []
            },
            {
              "id": "maggie",
              "name": "maggie (moved)",
              "children": []
            }
          ]
        }
      },
      "inverse": {
        "op": "remove",
        "path": "/children/0"
      }
    }
  ]
}

my workaround replaces the above operations with

{
  "processed": [
    {
      "patch": {
        "op": "move",
        "from": "/children/1",
        "path": "/children/0"
      },
      "inverse": {
        "op": "move",
        "from": "/children/0",
        "path": "/children/1"
      }
    },
    {
      "patch": {
        "op": "replace",
        "path": "/children/0/name",
        "value": "homer (moved)"
      },
      "inverse": {
        "op": "replace",
        "path": "/children/0/name",
        "value": "homer"
      }
    }
  ]
} 

Additional context

Are you willing to (attempt) a PR?

Maybe

xaviergonz commented 5 years ago

Sounds good to me, though I'm not sure how it would be pulled off since arrays don't really have a "swap" method, so it might be hard to generate such patches for a single operation.

By the way, does it only apply to arrays?

MiroslavGannoha commented 4 years ago

For me detach + patch add worked well as move operation substitute