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: v-model.number modifier is not properly typed #25575

Closed
4 of 7 tasks
aparajita opened this issue Jul 4, 2022 · 10 comments
Closed
4 of 7 tasks

bug: vue: v-model.number modifier is not properly typed #25575

aparajita opened this issue Jul 4, 2022 · 10 comments
Labels
package: vue @ionic/vue package type: bug a confirmed bug report

Comments

@aparajita
Copy link

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x
  • Nightly

Current Behavior

Given this:

<template>
  <IonInput
    v-model.number="options.androidMaxAttempts"
    type="number"
  />
</template>

<script setup lang="ts">
import { IonInput } from '@ionic/vue'

const options = reactive({
  androidMaxAttempts: 0
})
</script>

If you run vue-tsc, you get this error:

error TS2322: Type 'number' is not assignable to type 'string | boolean | undefined'.

86           v-model.number="options.androidMaxAttempts"
             ~~~~~~~

  node_modules/.pnpm/@ionic+vue@6.1.12/node_modules/@ionic/vue/dist/types/vue-component-lib/utils.d.ts:2:5
    2     modelValue?: string | boolean;
          ~~~~~~~~~~
    The expected type comes from property 'modelValue' which is declared here on type 'IntrinsicAttributes & Partial<{}> & Omit<Readonly<IonInput & InputProps> & VNodeProps & AllowedComponentProps & ComponentCustomProps, never>'

Note that Vue has no problem compiling the code and it runs correctly.

Expected Behavior

There should not be any errors. If you change IonicInput to input, vue-tsc does not report any errors.

Steps to Reproduce

This minimal app was created using your Getting Started Wizard. I have no idea if it will build or run, it is there just to demonstrate the typing bug.

npm i -g pnpm
git clone https://github.com/aparajita/v-model-bug
cd v-model-bug
pnpm i
node_modules/.bin/vue-tsc --noEmit

Code Reproduction URL

https://github.com/aparajita/v-model-bug

Ionic Info

Ionic:

   Ionic CLI : 6.20.1 (/Users/aparajita/Library/pnpm/global/5/.pnpm/@ionic+cli@6.20.1/node_modules/@ionic/cli)

Capacitor:

   Capacitor CLI      : 3.6.0
   @capacitor/android : 3.6.0
   @capacitor/core    : 3.6.0
   @capacitor/ios     : 3.6.0

Utility:

   cordova-res : 0.15.4
   native-run  : not installed globally

System:

   NodeJS : v18.4.0 (/Users/aparajita/.fnm/node-versions/v18.4.0/installation/bin/node)
   npm    : 8.12.1
   OS     : macOS Monterey

Additional Information

@liamdebeasi I think this is one for you.

@ionitron-bot ionitron-bot bot added the triage label Jul 4, 2022
@liamdebeasi
Copy link
Contributor

Thanks for the issue. Can you make your repo public?

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Jul 5, 2022
@ionitron-bot ionitron-bot bot removed the triage label Jul 5, 2022
@aparajita
Copy link
Author

@liamdebeasi Too funny! I created that repo with your handy online app creator and it made it private by default! 😂

It's public now, thanks for taking a look, this is breaking some of my build pipelines.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Jul 10, 2022
@aparajita
Copy link
Author

And sorry for taking so long to reply, for some reason I had turned off notifications from this repo. 🙄

@sean-perkins
Copy link
Contributor

@aparajita thanks for reporting this issue. I am able to reproduce and have been able to narrow it to our output targets interface: https://github.com/ionic-team/stencil-ds-output-targets/blob/bc8878f5ccd1890ec378a57f8e69a44b15213102/packages/vue-output-target/vue-component-lib/utils.ts#L3-L5

Updating the interface locally to:

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

resolves the issue. Input is one of our few components that accepts a number type directly for the value.

We may want to consider making the type a generic in the future, so that the Stencil build step can pass the type of the component, as the required signature for the v-model, i.e.:

export interface InputProps<T> {
    modelValue?: T; // optionally we can assign a default type for T if one is not set
}

@sean-perkins
Copy link
Contributor

Hello @aparajita can you test with this dev-build and let me know if the problem persists?

6.1.14-dev.11657574349.17cb208c

I've confirmed against your reproduction app + reproduction steps.

@aparajita
Copy link
Author

Yup, that fixed it, thank you! 🙏 You can close this issue.

@sean-perkins
Copy link
Contributor

Thanks for the follow-up! Still a little more work on my end. We will get the output targets PR reviewed, and then open a PR in Ionic framework to update the version of that package. This issue will auto-close once that PR lands.

aparajita added a commit to aparajita/capacitor-biometric-auth-demo that referenced this issue Jul 19, 2022
@aparajita
Copy link
Author

Hey @sean-perkins, any idea when this will land? Didn't make it into 6.1.15.

@sean-perkins
Copy link
Contributor

@aparajita tentatively in the next patch release (we aim for weekly releases).

@ionitron-bot
Copy link

ionitron-bot bot commented Aug 20, 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 Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Aug 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: vue @ionic/vue package type: bug a confirmed bug report
Projects
None yet
Development

No branches or pull requests

3 participants