search

dai-shi / proxy-compare

Compare two objects using accessed properties with Proxy
https://www.npmjs.com/package/proxy-compare
MIT License
283 stars 18 forks source link

Nested comparison #3

Closed joelmoss closed 3 years ago

joelmoss commented 3 years ago

Apologies if I'm missing something here, but shouldn't the following test pass?

const s1 = { a: { b: 'b', c: 'c' } }
const a1 = new WeakMap()
const p1 = createDeepProxy(s1, a1)
p1.a
expect(isDeepChanged(s1, { a: { b: 'b', c: 'c' } }, a1)).toBe(false)

s1 and the given objects are identical, so isDeepChanged should be false, and from my understanding, this is not a referential comparison.

Hopefully you can clear this up for me. thx

dai-shi commented 3 years ago

It's based on immutable (and shared?) state model, so it doesn't pass and it's intentional. It checks Object.is at the leaf node of affectted, which is .a in this case. Also, if it finds equal node at the intermediate node, we stop traversing even if it's not the leaf of affected. This principal improves performance.

(One regret: I shouldn't have named "Deep" which misleads deep-equal.)

joelmoss commented 3 years ago

One regret: I shouldn't have named "Deep" which misleads deep-equal.

Ah well that is definately where I got confused. thx for the explanation.

dai-shi commented 3 years ago

I would definitely rename them when I had a chance. (maybe when we introduce breaking changes in v2. no such plan yet.) Elaborating more in README would be helpful, if we got more attention to this library. Closing this issue, but suggestions are always welcome.