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

optionMergeStrategies.inject #5603

Closed
eddow opened this issue May 4, 2017 · 6 comments
Closed

optionMergeStrategies.inject #5603

eddow opened this issue May 4, 2017 · 6 comments

Comments

@eddow
Copy link

eddow commented May 4, 2017

Version

2.3.2

Reproduction link

https://jsfiddle.net/yywfw3cL/5/

Steps to reproduce

Make a component extend another one while both parent and child define an injection : the parent' injection is lost (overridden)

What is expected?

Injections to be merged

What is actually happening?

Injections override parent' ones


The injections initialisation looks for the injections own keys - so we cannot merge with a prototype like it is done in function mergeAssets or with "Other object hashes" : props/methods/computed
We instead have to copy the object and extend it in its own properties.
It might be cleaner if it was allowed to use the prototype.

@posva
Copy link
Member

posva commented May 4, 2017

Thanks for reporting it.

This should be an easy first contribution because it needs little to no knowledge of Vue. The file that needs to be modified is at https://github.com/vuejs/vue/blob/dev/src/core/util/options.js. Inject are arrays or object, it should be able to merge both. extend from shared/utils can (and prolly should) be used.
In case someone is looking for a first contribution 🙂

@eddow
Copy link
Author

eddow commented May 4, 2017

jaj, ok, got it ;)
Note: it's quite logical indeed, I almost did it
I have a question though : is the easiest way fork->pull request?

@LinusBorg
Copy link
Member

I have a question though : is the easiest way fork->pull request?

Yes.

@ryzokuken
Copy link

I would like to resolve this as my first contribution to the project.

@jkzing
Copy link
Member

jkzing commented Jun 15, 2017

@ryzokuken Already an opened PR there. :) (#5827)

@ryzokuken
Copy link

ryzokuken commented Jun 15, 2017 via email

@Kingwl Kingwl closed this as completed Jun 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants