Skip to content
This repository has been archived by the owner on Feb 7, 2024. It is now read-only.

Issue # 249: Cannot use @Provide and @ProvideReactive simultaneously. #281

Merged
merged 1 commit into from
Dec 19, 2019

Conversation

gmoneh
Copy link
Contributor

@gmoneh gmoneh commented Nov 15, 2019

This PR addresses the discussion for issue #249 .
There was a previous commit that aimed to address this issue (PR #264 ), but that actually broke it further.
The problem arises because both the @provide and @ProvideReactive decorators are competing to install a provide function in the component options. So whichever one is used first is the one who wins (because of the condition that was there to avoid doing it more than once).
The strategy taken in this fix is to have only one possible provide function, which is aware of both regular injections and reactive injections, and will produce the appropriate provide options object with both types of properties.
As it is now, this feature is broken in the 8.3.0 release, so hopefully this fix can be approved for a minor number release in the near future.
Thanks!

@kaorun343
Copy link
Owner

@gmoneh

Thanks for your contribution!
I'll merge this.

@kaorun343 kaorun343 merged commit 1a7ceb0 into kaorun343:master Dec 19, 2019
@gmoneh
Copy link
Contributor Author

gmoneh commented Jan 13, 2020

Hi, @kaorun343 , question... do you plan to do a release of the package that includes the code in this pull request? As it stands now, the 8.3.0 has this feature broken, so we're waiting eagerly for a new release to be put out with this fix.

Thanks!

@jchapian
Copy link

Please do a patch release as 8.3.0 is broken. 1a7ceb0 Fixes the issue.

@kaorun343
Copy link
Owner

Please check the latest version, v8.4.0, thanks.

@jchapian
Copy link

This is still broken. The case of sibling providers does not work. It'll just attempt to overwrite: https://github.com/kaorun343/vue-property-decorator/pull/281/files#diff-a542f5e3653e6b2d1bd03489368849b9R78-R80

example:

<ProviderOne>
  <ConsumerOne />
</ProviderOne>
<ProviderOne>
  <ConsumerOne />
</ProviderOne>

@gmoneh
Copy link
Contributor Author

gmoneh commented Feb 10, 2020

@jchapian I have another pull request pending that fixes some issues I ran into after the 8.4.0 release. Can you check if that change solves the issues you found too?

@jchapian
Copy link

@gmoneh - It does not fix the core of the issue... I made the same fix locally when I tested 8.4.0. What I found was that the provided values from ProviderOne in the above example would all come from the first instance. Your PR just seems to silence the issue.

We should take a look at how these injections are applied, taking special consideration to the hierarchy to which the injecting components belong.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants