Open jimmycallin opened 1 week ago
Probably this is because toMatchObject
is checking hasOwnProperty
https://github.com/vitest-dev/vitest/blob/a8299df2140d2a1a62bec704514fd7e2163c9b18/packages/expect/src/jest-utils.ts#L527-L527
It looks like proxy can intercept more things. For example, something like this can make "more equivalent" object as plain one and toMatchObject
and error diff work https://stackblitz.com/edit/vitest-dev-vitest-kuacmv?file=test%2Frepro.test.ts
Though this seems technically possible, I don' think people usually go this far for proxy implementation, so it might make sense to loosen toMatchObject
(or specifically subsetEquality
) to skip hasOwnProperty
check. It looks mostly harmless removing this line though I haven't tested.
expect.objectContaining({ bar: "foo" })
also requires hasOwnProperty
. So, if we change toMatchObject
, then we probably should change this too.
https://stackblitz.com/edit/vitest-dev-vitest-dr5jfq?file=test%2Frepro.test.ts
@jimmycallin Btw, can you explain your actual use case? We think either behavior seems fine (current one or https://github.com/vitest-dev/vitest/pull/6675), but knowing a concrete use case would help decisions. For example, is proxy coming from some well-known library or your specific implementation?
@jimmycallin Btw, can you explain your actual use case? We think either behavior seems fine (current one or #6675), but knowing a concrete use case would help decisions. For example, is proxy coming from some well-known library or your specific implementation?
Sure! It may be a bit esoteric, but we are using react-query which has a concept of tracked queries, which checks which properties you actually use to reduce rerenders in react. We have a wrapper around a useQuery function that adds a few properties, which naively would look something like this:
function useWrapper() {
const query = useQuery(...);
return {...query, additionalProperty: true }
}
Unfortunately the rest spread causes it to "trigger" each property, which nullifies the optimization. So we use a proxy to get around it:
function useWrapper() {
const query = useQuery({...});
return new Proxy(query, {
get(target, prop, receiver) {
if (prop === "additionalProp") {
return true;
}
return Reflect.get(target, prop, receiver);
},
});
}
Then we use react-testing-library to test the result of the hook.
Hope this makes sense!
Interesting, wrapping useQuery
sounds like a sensible use case.
How about defining getOwnPropertyDescriptor / ownKeys
? I guess it would be too tedious? (honestly I didn't even know such proxy handler exists :smile:)
Interesting, wrapping
useQuery
sounds like a sensible use case. How about defininggetOwnPropertyDescriptor / ownKeys
? I guess it would be too tedious? (honestly I didn't even know such proxy handler exists 😄)
We had that previously, but just changed to Proxy after I saw this discussion that Proxy is a lot more performant: https://github.com/TanStack/query/discussions/8091
Which is what triggered this issue. :)
What I was suggesting is to implement getOwnPropertyDescriptor
and ownKeys
of proxy:
function useWrapper() {
const query = useQuery({...});
return new Proxy(query, {
get(target, prop, receiver) {
if (prop === "additionalProp") {
return true;
}
return Reflect.get(target, prop, receiver);
},
getOwnPropertyDescriptor(target, prop) {
if (prop === 'additionalProp') {
return { configurable: true, enumerable: true };
}
return Reflect.getOwnPropertyDescriptor(target, prop);
},
ownKeys(target) {
return [...Reflect.ownKeys(target, prop), 'additionalProp']
},
});
}
But, I think https://github.com/TanStack/query/pull/8092 proves the point. If useQuery
implements only Proxy.get
, then Reflect.getOwnPropertyDescriptor
won't probably work for downstream proxy.
EDIT: Oh wait, maybe their useQuery
is fine because they have new Proxy(result, { get })
, so default getOwnPropertyDescriptor/ownKeys
can enumerate the ones from result
.
Describe the bug
It seems like toMatchObject doesn't work well with proxy objects, and won't call the handler to check equality.
Reproduction
System Info
Used Package Manager
yarn
Validations