-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
toMatchObject doesn't work well with Proxy objects #6672
Comments
Probably this is because vitest/packages/expect/src/jest-utils.ts Line 527 in a8299df
It looks like proxy can intercept more things. For example, something like this can make "more equivalent" object as plain one and exampleconst proxyObj = new Proxy(
{},
{
get(target, prop, receiver) {
if (prop === 'bar') {
return 'foo';
}
},
getOwnPropertyDescriptor(target, prop) {
if (prop === 'bar') {
return { configurable: true, enumerable: true };
}
return Reflect.getOwnPropertyDescriptor(target, prop);
},
ownKeys() {
return ['bar'];
},
}
);
// pass
expect(proxyObj).toMatchObject({ bar: 'foo' })
// error diff
// Object {
// - "bar": "not-foo",
// + "bar": "foo",
// }
expect(proxyObj).toMatchObject({ bar: 'not-foo' }) Though this seems technically possible, I don' think people usually go this far for proxy implementation, so it might make sense to loosen vitest/packages/expect/src/jest-utils.ts Lines 567 to 573 in a8299df
|
vitest/packages/expect/src/jest-asymmetric-matchers.ts Lines 148 to 156 in 2a50464
|
@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 |
We had that previously, but just changed to Proxy after I saw this discussion that Proxy is a lot more performant: TanStack/query#8091 Which is what triggered this issue. :) |
What I was suggesting is to implement 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 TanStack/query#8092 proves the point. If EDIT: Oh wait, maybe their |
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
The text was updated successfully, but these errors were encountered: