close #6097: Allow defining optional inject dependency with default values #6322
Conversation
src/core/util/options.js
Outdated
| @@ -270,12 +270,21 @@ function normalizeProps (options: Object) { | |||
| */ | |||
| function normalizeInject (options: Object) { | |||
| const inject = options.inject | |||
| const normalized = {} | |||
| let val | |||
src/core/util/options.js
Outdated
| } | ||
| } else if (isPlainObject(inject)) { | ||
| for (const key in inject) { | ||
| val = inject[key] |
| @@ -362,6 +362,23 @@ describe('Options provide/inject', () => { | |||
| expect(`Injection "baz" not found`).not.toHaveBeenWarned() | |||
| }) | |||
|
|
|||
| // Github issue #6097 | |||
| it('should not warn when injections cannot be found but have default value', () => { | |||
| const vm = new Vue({}) | |||
There was a problem hiding this comment.
Can you add also an injection with a value and check that it takes precedence?
src/core/util/options.js
Outdated
| if (Array.isArray(inject)) { | ||
| const normalized = options.inject = {} |
There was a problem hiding this comment.
you can keep this line (and move it outside of the if) and remove the last line options.inject = normalized
|
Fixed. I added a separate test for provided value taking precedence over default. |
|
Thanks, we'll wait for @yyx990803 to review it as well 🙂 |
| @@ -55,8 +55,15 @@ export function resolveInject (inject: any, vm: Component): ?Object { | |||
| } | |||
| source = source.$parent | |||
| } | |||
| if (process.env.NODE_ENV !== 'production' && !source) { | |||
| warn(`Injection "${key}" not found`, vm) | |||
| if (!source) { | |||
There was a problem hiding this comment.
Isn't if (!result[key]) easier to understand here?
|
No, it's not the same. You don't want to check the value of the injection, but it's presence. It would work |
|
@pdanpdan Oh, yes I missed falsy values. NVM |
|
@yyx990803 Hi, any chance you can take a look at this? Quasar Framework developers cannot upgrade to Vue 2.4.x until this PR gets in. Thanks! |
|
By chime in, I meant to give further feedback 😉 , upvoting a comment is enough to say +1, no need to comment I need this to upgrade , +1 or similar I don't remember seeing weex files getting updated, any idea why that weex PR got in the git history of this PR @pdanpdan ? |
|
I don't understand where did the weex commit come from. I'm on vacation till next Monday and I'll have to wait till then to get to a computer. |
|
Seems all open PR are in the similar situation. |
|
I just changed the PR to exclude the weex commit. |
…default values Allow to define inject as object with name and default properties. The default is used if no provide is found, and no warning is emitted. #6097
Allow to define inject as object with name and default properties. The default is used if no provide
is found, and no warning is emitted.
close #6097
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
devbranch for v2.x (or to a previous version branch), not themasterbranchfix #xxx[,#xxx], where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information:
In case of components that can work both as standalone and as children, it would be nice to be able to provide default values for the not provided dependencies.
It would also avoid the warning for missing inject in case this is one of the expected use case.
inject: Array | { [key: string]: string | Symbol | { name?: string | Symbol, default?: any } }