Skip to content
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

[vue-compat] globalProperty with function as value is always bound to instance proxy #4403

Closed
Fiona-SUN opened this issue Aug 20, 2021 · 9 comments · Fixed by #4873
Closed
Labels
feat: v2 compat 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. 🐞 bug Something isn't working
Projects

Comments

@Fiona-SUN
Copy link

Version

3.2.4

Reproduction link

https://github.com/Fiona-SUN/vue3-minimal-reproduction

Steps to reproduce

  1. update vue to 3.2.4, install @vue/compat of the same version
  2. install UI Components Vant 3.2.0
  3. Global Register Toast (eg: https://youzan.github.io/vant/v3/#/en-US/quickstart#cdn)
  4. use this.$toast.loading in .vue file

What is expected?

The project should run on Vue 3 with Vant.

What is actually happening?

Uncaught(in promise) TypeError: this.$toast.loading is not a function.project broken


PublicInstanceProxyHandlers function use Function.prototype.bind() cause Toast lost some properties.
if there have any other methos to resolve the problem instead of Function.prototype.bind().

@posva
Copy link
Member

posva commented Aug 20, 2021

Please report the bug in Vant's repository or provide a repro without Vant

@posva posva closed this as completed Aug 20, 2021
@LinusBorg
Copy link
Member

LinusBorg commented Aug 20, 2021

I felt like looking into this even though the repo wasn't really minimal with the added dependency.

It's an edge-case issue with the compat behaviour for app.config.globalProperties:

https://github.com/vuejs/vue-next/blob/4adc5042f92c384d9aec2d19e05c8bc2c74b2a93/packages/runtime-core/src/componentPublicInstance.ts#L359

  • in plain Vue 3, whatever we provide down, we leave untouched
  • in @vue/compat, if the value of a globalProperty is a function, we bind it to the instance proxy, in order to mimic the bound behavior of Vue.prototype.someGlobalProperty = function () { ... }

The repo is running in compat MODE: 3 - maybe we should disable this behaviour then? or provide a separate feature flag?

We should discuss it

@LinusBorg LinusBorg reopened this Aug 20, 2021
@LinusBorg LinusBorg changed the title [vue-compat] UI Components Vant: Fail to resolve this.$toast.loading [vue-compat] globalProperty with function as value is always bound to instance proxy Aug 20, 2021
@Fiona-SUN
Copy link
Author

Fiona-SUN commented Aug 23, 2021

Thanks for helping me to explain this problem clearer.

I think that globalProperty with function as value should be a common approach in dependency libraries.such as ElementUI ,Vant and so on.
Now most of us would like to migrate our project from Vue2 to Vue3(Migration Build),but the dependency package can not supported fully which makes me confused.Losts of code need to be rewrited.I don't think this is a good performance in version migration.

(Forgive me for my poor English expression skills~

@smileqwe
Copy link

I think this behavior should be fixed, because the official versions of vue2 and vue3 do not have this problem, which will lead to differences in the perception of migrated users

@LinusBorg LinusBorg added 🐞 bug Something isn't working 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. labels Aug 23, 2021
@LinusBorg
Copy link
Member

LinusBorg commented Aug 23, 2021

It is a bug, but it's an edge case, really. it only affects you if you pass a function that also has additional properties on it (which is not something people do quite often).

A plain function without additional properties on it will work just fine.

@Fiona-SUN
Copy link
Author

@LinusBorg Thanks for you helping!

I agree with this. Indeedly, a function has additional properties should be function extension. - but it is undeniable that convenient methods are more popular with people.

@benwills
Copy link

benwills commented Mar 3, 2022

I just ran into this same issue and it took me a couple hours to figure out that this is an unsupported edge case; I only figured it out after putting it together from this and some other GitHub issues.

I want to suggest updating the documentation to include a reference to this unsupported edge case, especially since the example in the Plugins documentation gives an example of passing data to the $translate function. While it does use Provide/Inject to finish the example, it's not explained that this is necessary when using a function that passes a variable.

I would also suggest mentioning this in the Api documentation for globalProperties as well:

@benwills
Copy link

benwills commented Mar 4, 2022

For those who run into this same situation, I have created a repository to show the scope/combinations of globalProperties and Provide/Inject and where they do and don't work when using the Composition API:

I would be happy to submit pull requests to both of the documentation pages I linked to above, adding notes at the bottom about the scope and usage limitations of globalProperties. Let me know if that's something you'd like for me to do, and if you have any suggestions/requirements for how I word/structure anything, etc.

@tony19
Copy link
Contributor

tony19 commented Mar 8, 2022

Until #4873 is merged, a quick workaround is to convert the function property into an object.

For Vant's Toast plugin that sets up a global $toast property (as in the @Fiona-SUN's case here):

app.use(Toast)

const toast = app.config.globalProperties.$toast
app.config.globalProperties.$toast = { ...toast } 👈

demo 1

For Axios:

const axiosInstance = axios.create({ withCredentials: true })
app.config.globalProperties.$axios = { ...axiosInstance } 👈

demo 2

@LinusBorg LinusBorg added this to Approved in Next Patch Mar 10, 2022
@yyx990803 yyx990803 moved this from Approved to Done in Next Patch Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: v2 compat 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. 🐞 bug Something isn't working
Projects
6 participants