-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
fix(runtime-core): allow spying on proxy methods #4216
Conversation
ba8fc35
to
b0bc208
Compare
This is now rebased on top of #4472 Would be glad to hear your thoughts @yyx990803 as landing this would unblock the ecosystem to move to jest v27 |
9f23312
to
2d063f8
Compare
Now that #4472 landed, this PR is rebased on master and ready to review. The fix is intended to unblock the migration of the ecosystem to jest 27, but it also fixes a more general issue with updating proxies via |
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 hope we can merge this soon!
Since Jest v26.6.1, the mock method changed (see this commit jestjs/jest@30e8020) to rely on `Object.defineProperty` in some cases. This breaks spying on proxy's methods, because even if Jest is properly calling `Object.defineProperty`, the cached value in the `get` section of the proxy is never updated, and the spy is in fact never used. This is easily reproducible as vue-next already uses a version of jest with these changes. This is blocking projects (like vue-test-utils-next and vue-cli) to update to recent Jest versions. This commit adds a `defineProperty` method to the proxy handler, that properly updates the defined value in the cache.
2d063f8
to
c4c9903
Compare
Since Jest v26.6.1, the mock method changed (see this commit jestjs/jest@30e8020) to rely on
Object.defineProperty
in some cases.This breaks spying on proxy's methods, because even if Jest is properly calling
Object.defineProperty
, the cached value in theget
section of the proxy is never updated, and the spy is in fact never used.This is easily reproducible as vue-next already uses a version of jest with these changes.
This is blocking projects (like vue-test-utils-next and vue-cli) to update to recent Jest versions.
This commit adds a
defineProperty
method to the proxy handler, that properly updates the defined value in the cache.Again, the fix is maybe too naive, and I'm happy to update the PR with a better solution.
edit: the same happens with Vitest