Test fails with undefined property

[aloscha]

Two tests fails with undefined as a default property.
First test:
it('undefined property is set to something', function() {
var obj = {foo: undefined};

      var observer = jsonpatch.observe(obj);
      obj.foo = "something";

      var patches = jsonpatch.generate(observer);
      expect(patches).toEqual([{op: 'replace', path: '/foo', value: "something"}]);
    });

IMHO there need to be 'add' instead of 'replace'

Second test:

  it("should generate remove when `undefined` property get removed", function(){
    var obj = {foo: undefined};

    var observer = jsonpatch.observe(obj);
    delete obj.foo;

    var patches = jsonpatch.generate(observer);
    expect(patches).toEqual([{ op: 'remove', path: '/foo' }]);
  });

Here JSON-Patch create empty array as a result.

Also In that task needs to refactor readme:

README says: "We treat undefined as any other falsy value, and
implementation & tests works: "We tread undefined as no property specified"

@tomalec, @warpech what guys do you think about it?

[tomalec]

There are many issues related to undefined

Some time ago I have tried to add support for undefined as any other falsy value, but it seems it no longer works this way.

If i's to hard to fix JSON-Patch to work that way once again, I would re-describe applicable test cases, and add note on README what is the current behavior - make README in sync with spec.

[warpech]

Also, in NodeJS test case 13 tests fail:

[aloscha]

@warpech that happens before #104 ? (I mean it's happens because changes in that task occurs)?

[warpech]

@warpech that happens before #104 ? (I mean it's happens because changes in that task occurs)?

Yes

[aloscha]

@warpech I fixed that, by start using jsdom. Now node test returns only 2 errors (the same like in SpecRunnerDuplex.html)

[warpech]

Thanks @aloscha

[aloscha]

If i's to hard to fix JSON-Patch to work that way once again, I would re-describe applicable test cases, and add note on README what is the current behavior - make README in sync with spec.

Ok, waiting for that

[warpech]

Back to the original subject - first of all, we should stop using Jasmine's method toEqual, because it thinks the following statement is correct:

expect({a: undefined}).toEqual({});

Secondly, I propose to use the following approach. jsonpatch.generate should treat undefined exactly the same way it is treated in JSON.stringify and jsonpatch.compare.

Every test should go the following path (pseudocode):

var a = /*some object*/;
observe(a);
a.change = /*new value*/;
var patches1 = generate(a);

var b = /*same object as "a"*/;
var c = clone(b);
b.change = /*same value as "a.change"*/
var patches2 = compare(b, c);

expect(patches1).toReallyEqual(/*expected patch*/);
expect(patches1).toReallyEqual(patches2);

The following commit implements that: ffaa563

@tomalec WDYT?

In that commit, I don't use Jasmine's toEqual, but Underscore's isEqual (wrapped in a matcher called toReallyEqual).

[warpech]

I created a pull request. This is a breaking change for undefined.

@tomalec, @miyconst, @aloscha could you please review the PR? There is a chance that I have overlooked some problem, so please be very critical :)

[warpech]

Changes are now merged to master.