JSONPatcherProxy icon indicating copy to clipboard operation
JSONPatcherProxy copied to clipboard

Filtering out non-significiant changes Palindrom behavior

Open tomalec opened this issue 6 years ago • 1 comments

@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

tomalec avatar Sep 05 '19 08:09 tomalec

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.

warpech avatar Sep 05 '19 10:09 warpech