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

bug: Vue output modelValue is required #233

Closed
markbrockhoff opened this issue Mar 2, 2022 · 5 comments · Fixed by #234
Closed

bug: Vue output modelValue is required #233

markbrockhoff opened this issue Mar 2, 2022 · 5 comments · Fixed by #234
Assignees
Labels
package: vue @stencil/vue-output-target package type: bug Something isn't working

Comments

@markbrockhoff
Copy link
Contributor

markbrockhoff commented Mar 2, 2022

Hello,

I noticed that the generated types for Vue always require the property v-model to be defined.
This becomes especially noticeable when using tools like vue-tsc or volar for typechecking a vue project.

image

From my understanding, this comes from the interface InputProps defined in the generated file vue-component-lib/utils.ts.
Simply making the property modelValue optional seemed to fix the problem in my testing.

export interface InputProps extends Object {
  modelValue?: string | boolean;
}

I already created a small PR to implement the suggested fix: #234

@sean-perkins sean-perkins changed the title bug: modelValue is required bug: Vue output modelValue is required Mar 2, 2022
@sean-perkins sean-perkins added package: vue @stencil/vue-output-target package type: bug Something isn't working labels Mar 2, 2022
@sean-perkins
Copy link
Collaborator

Hello @markbrockhoff thanks for the issue and the PR! I agree that the type is too strict and should be optional in this case.

Would you be able to create a simple reproduction app with one of the problematic dependencies that conflicts with the types? This will help in my ability to confirm your PR resolves the problem correctly and can help improve the review/merge time.

Thanks!

@markbrockhoff
Copy link
Contributor Author

markbrockhoff commented Mar 3, 2022

Hi @sean-perkins thanks for the quick response.
Sure, I created a small demo repo containing the stencil-component-starter, the vue-output-target and a simple vue 3 app showing the problem: https://github.com/markbrockhoff/stencil-component-starter
There's a small readme and a demo bash script that sets everything up for you and reproduces the error. Im using vue-tsc to check the types of the vue components when building the vue app. It's that step where the required property gets noticeable.

@sean-perkins
Copy link
Collaborator

Thanks @markbrockhoff! I'll try to clone and validate this weekend, Monday at the latest.

@sean-perkins sean-perkins self-assigned this Mar 4, 2022
sean-perkins added a commit that referenced this issue Mar 9, 2022
@markbrockhoff
Copy link
Contributor Author

markbrockhoff commented Mar 10, 2022

@sean-perkins Do you know when this fix will be released? It'd be great if we could install and use the new version as soon as possible.

@ionitron-bot
Copy link

ionitron-bot bot commented Apr 15, 2022

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of the output targets, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Apr 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: vue @stencil/vue-output-target package type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants