inspect-js / node-deep-equal

node's assert.deepEqual algorithm
MIT License
777 stars 109 forks source link

Weird Compare Fail #103

Closed AtesComp closed 11 months ago

AtesComp commented 1 year ago

I have a case where comparing two object directly fails:

deepEqual( this.resource, resourceEdit )  --> false

but comparing them after processing through JSON.parse( JSON.stringify() ) succeeds:

deepEqual(
  JSON.parse( JSON.stringify(this.resource) ),
  JSON.parse( JSON.stringify(resourceEdit) ) )  --> true

Any ideas? I can provide the example, but since I dump it using JSON.stringify(), I can't produce the failure out of code.

ljharb commented 1 year ago

a JSON roundtrip tells you nothing, because JSON only supports a tiny subset of JS values.

What I usually do to find out why things aren't equal is use https://npmjs.com/is-equal, and is-equal/why, which will return a string describing why they differ.

AtesComp commented 1 year ago

Testing with is-equal/why is equally uninformative:

console.debug("DeepEqual: " + deepEqual(this.person, personEdit) );
console.debug("WhyNotEqual: " + (whyNotEqual(this.person, personEdit) || "EQUAL"));

produces

DeepEqual: false
WhyNotEqual: EQUAL

Any other suggestions?

ljharb commented 1 year ago

oh hm, interesting. What's the result of:

console.log(Object.getPrototypeOf(this.person), Object.getPrototypeOf(personEdit));
console.log(Reflect.ownKeys(this.person), Reflect.ownKeys(personEdit));

?

AtesComp commented 1 year ago

Thanks for the help. Based on your hint and that I'm using Vue and the objects are reactive, there seems to be a reactivity issue. In it, there also seems to be an object recursion issue in the reactive part as well.

Essentially, since the data fits within the scope of a JSON structure, using the above JSON solution works at it removes the reactivity. However, I am interested in a more robust solution. I'm attempting to remove the reactivity via a deep clone process that ignores the reactive parts. You may know some other solution.

Is "whyNotEqual" (and "isEqual") doing a deep compare (or does it depend on "deepEqual")? If not, that obviously explains the difference. If so, then the reactive components must be resolving to the values with "whyNotEqual" and not resolving when using "deepEqual". Either that or the recursion is factor. At least, that's my current theory.

ljharb commented 1 year ago

When you say “reactive”, do you mean, one is wrapped in a proxy and the other isn’t? Since true reactivity isn’t a feature JavaScript can possibly provide, you’ll run into all sorts of edge cases when trying to use it. There isn’t any better solution than avoiding a proxy-based pattern entirely.

whyNotEqual and deepEqual are entirely separate, and have their own, slightly different algorithms. However, if the identity is different due to proxy wrapping, I’d expect both to report the same.

AtesComp commented 1 year ago

They are both reactive. The Vue reactive objects are unavoidable--they need to be reactive for other processing. It does provide a method to get the non-reactive values, but that can be problematic as well since it does not recurs.

It is very clear from my tests that, for whatever difference there is between whyNotEqual and deepEqual, the reaults are not the same.

For Vue-based projects, something like the following can be done to expose a raw value:

if ( isRef(obj) || isReactive(obj) || isProxy(obj) ) return toRaw(obj);

A major issue is that the so-called "reactivity" is a processing "compile" step that wraps the original variable value in a same named variable with some "value" key to hold the value. Other elements are added to manage reactivity. This is generally hidden from the programmer when writing the code, but implied throughout. Then, this also complicates any additional processing like using deepEqual, etc. The system is supposed to unwrap these values under some circumstances, but this is not clear at all. I take it as variables are generally wrapped / proxy / reactive / whatever when you declare them in certain parts of the code or explicitly tell it to do so.. Vue "may" be unwrapping the top reactive object element for the compare, but I have very little confidence it unwraps any additional embedded sub-objects.

So, my understanding is that when I write a = b, Vue will likely transcode that to something like a.value = b.value and extra code to identify the update to a. This easily lulls one to write deepEqual(a, b) without understanding any unforeseen consequences or side-effects. I have no control over what Vue adds to the mix to cause failure of an otherwise equal object. Add to that the extremely bad documentation for all of these frameworks (documentation seems to be a far afterthought).

Essentially, I need to process a deep recursive toRaw on both variables to create new variables before the deepEqual process. Then, I ask myself, what's the point if I need to recurs the objects anyway? I did do this as a test case and it "seems" to work, but what guarantee is there that it actually compared what I intended to compare with all the cruft?

ljharb commented 1 year ago

isProxy isn't a check the language allows either (altho node exposes one), how do you check that?

AtesComp commented 1 year ago

The isRef(), isReactive(), isProxy() and toRaw() functions are all provided by VueJS. See for example: https://vuejs.org/api/reactivity-advanced.html#toraw

Also: https://vuejs.org/api/reactivity-utilities.html

ljharb commented 1 year ago

ahhh ok, so it's how vue lets you determine if vue-created proxies are proxies.

ok so then yes, if these are two different vue proxies for the same object, there's no way for a non-vue-specific comparison library to give you meaningful data about them. It's very bizarre that those two packages give a different answer, but it's likely due to the is-equal package treating distinct functions that have the same toString output as equal, while everything else in the ecosystem does not check the toString output.

AtesComp commented 1 year ago

Then, cool. I'll switch over to using is-equal and test.

For speed, what do you think is faster:

  1. is-equal directly on unaltered proxy objects
  2. toRaw proxy removed recursively on new objects, then is-equal

Speed will likely not be a significant factor for my use case.

I'm thinking that toRaw processed objects could be, er, "safer" JS language-wise.

ljharb commented 1 year ago

I'd do the toRaw, part, yes - Proxy isn't something you want to be dealing with if you can avoid it :-)