Palindrom / JSONPatcherProxy

ES6 proxy powered JSON Object observer that emits JSON patches when changes occur to your object tree.
94 stars 14 forks source link

Inspect code for bugs that are not covered by tests #40

Open warpech opened 5 years ago

warpech commented 5 years ago

After https://github.com/Palindrom/JSONPatcherProxy/pull/39

The code in https://github.com/Palindrom/JSONPatcherProxy/blob/c461b492a4fa2e2869ba99cbe0329db3846e93cd/src/jsonpatcherproxy.js contains several TODO statements that mark places for functionality that is not tested.

Also check why running the test suite results in so many console loigs. Is this by design, or are some of these console logs actually pointing out a problem:

λ npm run test

> jsonpatcherproxy@0.0.10 test W:\repo\JSONPatcherProxy
> jasmine DUPLEX=proxy JASMINE_CONFIG_PATH=test/jasmine.json

Started
...........You're accessing an object that is detached from the observedObject tree, see https://github.com/Palindrom/JSONPatcherProxy#detached-objects
..JSONPatcherProxy noticed a non-integer prop was set for an array. This will not emit a patch
JSONPatcherProxy noticed a non-integer prop was set for an array. This will not emit a patch
.....JSONPatcherProxy noticed a non-integer prop was set for an array. This will not emit a patch
JSONPatcherProxy noticed a non-integer prop was set for an array. This will not emit a patch
.........................You're accessing an object that is detached from the observedObject tree, see https://github.com/Palindrom/JSONPatcherProxy#detached-objects
You're accessing an object that is detached from the observedObject tree, see https://github.com/Palindrom/JSONPatcherProxy#detached-objects
.......
warpech commented 5 years ago

Regarding the TODO in the line https://github.com/Palindrom/JSONPatcherProxy/blob/c461b492a4fa2e2869ba99cbe0329db3846e93cd/src/jsonpatcherproxy.js#L280, do not miss the comment in https://github.com/Palindrom/JSONPatcherProxy/pull/39#discussion_r291162813