-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reflect to proxy object, to observe changes made by setters #41
Conversation
We can argue, that JS object with setter is not a JSON, therefore, we should not compromise performance for non-standard object. |
React Easy State does not make this sacrifice. See "Avoid using the this keyword in the methods of your state stores." |
observedObj = JSONPatcherProxy.deepClone(observedObj); | ||
|
||
expect(obj2).toReallyEqual(observedObj); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think lined 200-210 are redundant in this test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well they check if the patch application leads to the correct result. However they are quire redundant with fast-json-patch
tests.
It was discussed on a HO between me and @tomalec that we should introduce 2 modes of operation in JSONPatcherProxy:
I can volunteer to do this change, but only if PR #42 is accepted first. Otherwise I would go nuts with the tests :) |
"should generate replace (changed by setter)", as suggested at https://github.com/Palindrom/JSONPatcherProxy/pull/41/files/f934dc50b8ae31f33135d7e45cc1c1a2aebaceed#r296681603 change `var` to `const`
This PR was rebased on current |
For example, to be able to observe the change made to
obj.foo
by settingobj.bar='new'
Before this PR,
this
in setter was referring to original object not the proxy.This change decreases performance significantly. According to
npm run bench
executed 3 times on my local it's: