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.

Sign up for GitHub

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 #6008, Add merge strategy for provide option #6025

Merged
merged 4 commits into from
Jul 7, 2017
Merged

Fix #6008, Add merge strategy for provide option #6025

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6300551
test: Provide option not getting merged from mixins
znck Jul 4, 2017
e23224b
fix: Merge strategy for provide option
znck Jul 4, 2017
6c7c334
test: provide can be function or object
znck Jul 4, 2017
80bf22f
fix: use data merge stategy for provide option
znck Jul 5, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/core/util/options.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ strats.watch = function (parentVal: ?Object, childVal: ?Object): ?Object {
strats.props =
strats.methods =
strats.inject =
strats.provide =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually not enough because provide can also be a function that returns an object.

I think we should make data strategy reusable and use it for provide
Other plugins like vuefire will also benefit from this 🙂

Copy link
Member Author

@znck znck Jul 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally missed that. 😝 I'd add a failing test first.

strats.computed = function (parentVal: ?Object, childVal: ?Object): ?Object {
if (!childVal) return Object.create(parentVal || null)
if (!parentVal) return childVal
Expand Down
40 changes: 40 additions & 0 deletions test/unit/features/options/inject.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,4 +298,44 @@ describe('Options provide/inject', () => {
expect(`Injection "bar" not found`).not.toHaveBeenWarned()
expect(`Injection "baz" not found`).not.toHaveBeenWarned()
})

// Github issue #6008
it('should merge provide from mixins', () => {
const mixinA = { provide: { foo: 'foo' }}
const mixinB = { provide: { bar: 'bar' }}
const child = {
inject: ['foo', 'bar'],
template: `<span/>`,
created () {
injected = [this.foo, this.bar]
}
}
new Vue({
mixins: [mixinA, mixinB],
render (h) {
return h(child)
}
}).$mount()

expect(injected).toEqual(['foo', 'bar'])
})
it('should merge provide from mixins and override existing keys', () => {
const mixinA = { provide: { foo: 'foo' }}
const mixinB = { provide: { foo: 'bar' }}
const child = {
inject: ['foo'],
template: `<span/>`,
created () {
injected = [this.foo]
}
}
new Vue({
mixins: [mixinA, mixinB],
render (h) {
return h(child)
}
}).$mount()

expect(injected).toEqual(['bar'])
})
})