Fix json-patch check #103 #98

[aloscha]

Remove slowCheck (dirty checking in fixed time intervals) #103
Change the observed events to mouseup, keyup #98

[tomalec]

I'd replace it with just

observer.next = setTimeout(dirtyCheck);

[tomalec]

@aloscha Have you tested it with some Polymer binding example (for example PuppetJs demo, or something with arrays - Polymer acts more asynchronously for bound arrays) that we do not need another single check after some delay?

[aloscha]

I've tested it with KitchenSink, whoch is used PuppetJs

[warpech]

Reassigining to @tomalec for code review.

[tomalec]

This seems to be not correct.
Test code does not match description:

should generate replace, when undefined property is set to something

expect(patches).toEqual([{op: 'add', path: '/foo', value: "something"}]);

[aloscha]

should generate replace, when undefined property is set to something

Yeah but for me it's strange, for example object.property = undefined, but if we doesn't assign to it undefined and write into console object.property, we also receive undefined.

So it's definitely add instead of replace

[tomalec]

I have added my comments at https://github.com/Starcounter-Jack/JSON-Patch/pull/104/files

[tomalec]

Also if you are changing spec for undefineds please make sure to keep it consistent with README: https://github.com/Starcounter-Jack/JSON-Patch#undefineds-json-to-js-extension

[tomalec]

In general I would suggest not to mix undefined related things to this PR, you can submit them independently, referring other applicable issues.

[warpech]

Also if you are changing spec for undefineds please make sure to keep it consistent with README: https://github.com/Starcounter-Jack/JSON-Patch#undefineds-json-to-js-extension

@aloscha's problem was that the tests for undefineds were not passing. So the changes were supposed only to fix the test.

[tomalec]

1. I would change it in separate issue and commit
2. I would like to avoid the case when
   • README says: "We treat undefined as any other falsy value, and
   • implementation & tests works: "We tread undefined as no property specified"

[aloscha]

@warpech indeed.
@tomalec

1. Ok so I rollback changes with undefined, create issue and reintroduce changes
2. But how is it possible to commiet tests, which fail solution?

[aloscha]

I've remove tests with undefinedand create new issue - #105

@tomalec could you look at it again?

[tomalec]

LGTM, could you check if it passes the test suite with PuppetJs/puppet-polymer-client? If so, we can marge this PR

[aloscha]

@tomalec It passes