Add provided/inject support for Composition API components #1354
Conversation
|
The latest build fail is flow issues in |
test/resources/components/component-with-inject-composition.vue
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,4 @@ | |||
| { | |||
There was a problem hiding this comment.
Probably not a great idea. We should not keep such files in the repo.
There was a problem hiding this comment.
Hmmm... I'm not a VSCode user. I can go download VSCode and try to setup our repo with it, but from googling, it seems like this file is needed for flow https://code.visualstudio.com/docs/languages/javascript#_can-i-use-other-javascript-tools-like-flow
There was a problem hiding this comment.
I had to configure this myself too when setting up the local env yesterday. Not sure if committing it here is the best approach (I've done so in my projects), but it's gonna be needed for every VSC user.
There was a problem hiding this comment.
Thank you so much for this PR 🎉! Two small changes and one question:
- Can you place the composition API in the devDependencies?
- Can you assert that
setInBeforeCreateis equal tocreatedin your test?
Question: Would it be helpful for you as a VSCode user if the settings.json file was there when you first cloned the project?
| @@ -0,0 +1,4 @@ | |||
| { | |||
There was a problem hiding this comment.
Hmmm... I'm not a VSCode user. I can go download VSCode and try to setup our repo with it, but from googling, it seems like this file is needed for flow https://code.visualstudio.com/docs/languages/javascript#_can-i-use-other-javascript-tools-like-flow
test/resources/components/component-with-inject-composition.vue
Outdated
Show resolved
Hide resolved
|
The tests are failing due to some |
|
Should be good now @JessicaSachs |
|
I think the flow upgrade also caused a failure... Lemme revert the flow bin locally and test what happens |
|
Looks like flow errors? |
|
Yeah, pin the version to 0.66.0 and re-run yarn install. That fixes the errors for me. |
|
|
^0.66.0 Will be fine for now. I'm probably going to pin it at 0.66.0 after the merge, until we have a chance to fix the Element issues so someone doesn't accidentally upgrade it. |
|
Hey @Stoom, thank you for your time and effort spent on this PR, contributions like yours help make Vue better for everyone. Cheers! 💚 |
| !injectSupported || mountingMethod.name === 'renderToString', | ||
| 'supports setup in composition api component', | ||
| () => { | ||
| if (!injectSupported) return |
There was a problem hiding this comment.
What is the point of this? It is already checked above?
| localVue | ||
| }) | ||
|
|
||
| expect(wrapper.vm.setInSetup).to.equal('created') |
There was a problem hiding this comment.
How is this connected to provide/inject?
It looks like with the new composition api components that the provider changed from
vm.providetovm._provided. This PR adds both so that composition apis that use injection can be tested.