-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 regression #5415 #4216 #5417
fix(runtime-core): allow spying on proxy methods regression #5415 #4216 #5417
Conversation
✔️ Deploy Preview for vuejs-coverage ready! 🔨 Explore the source changes: 834965a 🔍 Inspect the deploy log: https://app.netlify.com/sites/vuejs-coverage/deploys/620980bef58030000761bb13 😎 Browse the preview: https://deploy-preview-5417--vuejs-coverage.netlify.app |
✔️ Deploy Preview for vue-next-template-explorer ready! 🔨 Explore the source changes: 834965a 🔍 Inspect the deploy log: https://app.netlify.com/sites/vue-next-template-explorer/deploys/620980be4d0f24000730dc2f 😎 Browse the preview: https://deploy-preview-5417--vue-next-template-explorer.netlify.app |
✔️ Deploy Preview for vue-sfc-playground ready! 🔨 Explore the source changes: 834965a 🔍 Inspect the deploy log: https://app.netlify.com/sites/vue-sfc-playground/deploys/620980be036016000726ab28 😎 Browse the preview: https://deploy-preview-5417--vue-sfc-playground.netlify.app/ |
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.
Thanks @lidlanca
This looks good to me, I just added a few comments.
packages/runtime-core/__tests__/componentPublicInstance.spec.ts
Outdated
Show resolved
Hide resolved
packages/runtime-core/__tests__/componentPublicInstance.spec.ts
Outdated
Show resolved
Hide resolved
packages/runtime-core/__tests__/componentPublicInstance.spec.ts
Outdated
Show resolved
Hide resolved
this.set!(target, key, descriptor.get(), null) | ||
} else if (descriptor.value != null) { | ||
// invalidate key cache of a getter based property #5417 | ||
target.$.accessCache[key] = 0; |
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.
quasarframework/quasar#13154
Under what circumstances can accessCache
be undefined?
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.
In our case (Quasars q-input component) the problem is, that target.$
is undefined
The error message was:
TypeError: Cannot read properties of undefined (reading 'accessCache')
at Object.defineProperty (runtime-core.esm-bundler.js:6823:1)
at Function.defineProperty (<anonymous>)
at injectProp (inject-obj-prop.js:2:10)
at use_validate (use-validate.js:210:13)
at use_field (use-field.js:173:7)
at setup (QInput.js:395:30)
at callWithErrorHandling (runtime-core.esm-bundler.js:155:1)
at setupStatefulComponent (runtime-core.esm-bundler.js:7072:1)
at setupComponent (runtime-core.esm-bundler.js:7028:1)
at mountComponent (runtime-core.esm-bundler.js:4937:1)
a getter should not get evaluated when defining a property using
defineProperty
instead we invalidate the key
accessCache
fixes: #5415 #4216