search

json-patch / json-patch-tests

Tests for implementations of json-patch
68 stars 21 forks source link

Test the removal of the whole document #29

Open espadrine opened 8 years ago

espadrine commented 8 years ago

See also https://github.com/json-patch/json-patch2/issues/17

sonnyp commented 8 years ago

replacing the document with null is not removing the document

I believe the behavior of removing / should be left to the implementation as it cannot be semantically defined by json patch

In https://github.com/JSON8/patch I return undefined

espadrine commented 8 years ago

In https://github.com/JSON8/patch I return undefined

While that is compliant with the spec, it is unfortunate, as it breaks the expectation that the result remains a valid JSON document. I would recommend you change it to return null.

I believe the behavior we want should be explicit rather than implicit or unspecified. The spec defines the semantics of all that it defines. In the meantime, having divergence in implementations' behavior is unfortunate.

sonnyp commented 8 years ago

JSON.stringify(undefined) returns "undefined" so the program won't break and it let the user decides what to do for the remove / case.

Anyway, this repo contains JSON Patch tests and since your proposed behavior is not specified I'd :-1:

espadrine commented 8 years ago

I'm not fond of the idea that removing the object converts it to the 9-character string "undefined" by default.