cujojs / jiff

JSON Patch and diff based on rfc6902
Other
627 stars 41 forks source link

Improper patch creation when changing an array to an object #29

Closed jflitton closed 9 years ago

jflitton commented 9 years ago

First off, thanks for this wonderful library! I've written an event store which uses it and I've encountered the following behavior which I believe to be a bug:

var a = {
  "stuff": ['x']
};

var b = {
  "stuff": {
    "a": "x"
  }
};

var patch = jiff.diff(a,b);

results in a patch like this:

[
  {
    "op": "add"
    "path": "/stuff/a"
    "value": "x"
  }
  {
    "op": "test"
    "path": "/stuff/0"
    "value": "x"
  }
  {
    "op": "remove"
    "path": "/stuff/0"
  }
]

This patch cannot be applied however because the path "/stuff/a" isn't valid for an array:

c = jiff.patch(patch, a);  // SyntaxError: invalid array index a

The patch I expected to see is:

[
  {
    "op": "replace"
    "path": "/stuff"
    "value": {
      "a": "x"
    }
  }
]

Am I way off base here?

briancavalier commented 9 years ago

@jflitton Thanks for the detailed report. That sure seems like a bug! I should have time to look into it this weekend. I bet the problem is that our isValidObject check is too lax and causes this code branch to allow an Array and an object. Should be super-easy to fix.

briancavalier commented 9 years ago

Yep, that was it. Could you try the fix-issue-29 branch to see if it works for you? Thanks!

jflitton commented 9 years ago

@briancavalier that did the trick! Thanks for the quick fix.

briancavalier commented 9 years ago

Cool, thanks for confirming. Version 0.7.2 just landed with this fix :confetti_ball: