search

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

Filtering out non-significiant changes Palindrom behavior #58

Open tomalec opened 5 years ago

tomalec commented 5 years ago

@warpech, @eriksunsol https://github.com/Palindrom/JSONPatcherProxy/pull/45 changes Palindrom behavior in respect of numbers > Number.MAX_SAFE_INTEGER. This change checks that Number.MAX_SAFE_INTEGER == Number.MAX_SAFE_INTEGER +1 // true and do not emit a patch, therefore such error is not cough by Palindrom's validation. And the tests that are failing are: 

HTTP :
WebSocket:
Outgoing patch: out of range numbers should call onOutgoingPatchValidationError event with a RangeError

WDYT of

  1. adding Number range validation to JSONPatcherProxy?
  2. Removing Range validation for outgoing patches from Palindrom?

as I cannot think of any other solution.

You can check the behavior at https://github.com/Palindrom/Palindrom/compare/update-deps?expand=1 https://travis-ci.org/Palindrom/Palindrom/builds/581106763

warpech commented 5 years ago

I vote for option 2 with clarification in JSONPatcherProxy and Palindrom docs.

Reasoning: Anyone who uses numbers above MAX_SAFE_INTEGER in JavaScript should expect the unexpected. Since JS can't recognize a difference between two numbers, I don't think that JSONPatcherProxy or Palindrom should do that either.

Option 1 would be a performance degradation.