Skip to content

defineModel ignores TS types #9587

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
T0miii opened this issue Nov 11, 2023 · 5 comments · Fixed by #9603
Closed

defineModel ignores TS types #9587

T0miii opened this issue Nov 11, 2023 · 5 comments · Fixed by #9603
Labels
🐞 bug Something isn't working scope: compiler

Comments

@T0miii
Copy link

T0miii commented Nov 11, 2023

Vue version

3.3.8

Link to minimal reproduction

https://play.vuejs.org/#eNqFUstOwzAQ/JXFlxapNEK9VWklQJUAiYcAwcWXNN2mKY5t+VGKQv6dtUNDQTxO9s7semc9W7MTrYcbj2zMUpubUjuw6LwGkcliwpmznE25LCutjIMaDC6hgaVRFfSorNdRZ6rSLc7ZMAlReJYzLrnMlbQOKlvAJDzQ752jEAqelBGLg94hl2nS9qZOFDistMgcUgSQro6ndR2LmyZNKIpobLc5qtQCBckkmjNIiEuTrpwNSD71XpbFcG2VpBnrUMxZTtWlQHOjXUnaOBtDZAKXkbSXy4g543Gww/MV5s8/4Gu7DRhntwYtmg3N3HEuMwW6lp7dX+OW7h1J2r2g7D/IO7RK+KCxTTv1ckGy9/Ki2otoQSmLBzvbOpR2N1QQGjKbmM8ZORI+7rfRP+WOhqNYx2VDv7hz858l+bA5WPKYCY/k9gKXpcSrAKXWGZIIbyB9NUcTL0LQMVdKYCan/a+b8G0PSqm9A/eqMXSMv7W/AF3XKGV/C5p3e9v5Lg==

Steps to reproduce

Create a Component with a defineModel that got multiple types

<script setup lang="ts">
const modelValue = defineModel<string | number | null | boolean>()
</script>
<template>
  <input type="text" v-model="modelValue">
</template>

Use the component.

<script setup lang="ts">
import { ref } from 'vue'
import Comp from "./Comp.vue"

const msg = ref('Hello World!')
</script>

<template>
  <h1>{{ msg }}</h1>
  <Comp v-model="msg" />
</template>

Observe warning:

image

What is expected?

Expected is that the component accepts the string without any errors.

What is actually happening?

The component throws a warning that it got passed a String instead a Boolean

System Info

No response

Any additional comments?

Im now defineModel is an experimental feature , but i guess the user would expect that it works as the usual prop type definition that throws no warning.

@LinusBorg
Copy link
Member

LinusBorg commented Nov 11, 2023

I think this is related to special handling for boolean props that we also have in other scenarios.

When we define props with a type interface, we generate the prop runtime definition without a runtime type (because the component will provide TS types to devs):

const modelValue = defineModel<string | number | null>()

// results in runtime prop def:
  props: {
    "modelValue": {},
  },

But when the prop is of type Boolean, we need to know that during runtime for casting props values properly.

const modelValue = defineModel<boolean>()

// results in runtime prop def:
  props: {
    "modelValue": { type: Boolean },
  },

Problem is that we get the same result for a union type containing boolean like in the repro:

const modelValue = defineModel<string | number | null | boolean>()

// results in runtime prop def:
  props: {
    "modelValue": { type: Boolean },
  },

It would need to be:

const modelValue = defineModel<string | number | null | boolean>()

// results in runtime prop def:
  props: {
    "modelValue": { type: [Boolean, String, Number] },
  },

@LinusBorg LinusBorg added 🐞 bug Something isn't working scope: compiler labels Nov 11, 2023
@yangxiuxiu1115
Copy link
Contributor

I briefly reviewed the source code and it looks like it was intentionally done. In the product environment, supports Boolean and Function when options are passed in.

runtimeTypes = runtimeTypes.filter(el => {
if (el === UNKNOWN_TYPE) return false
return isProd
? el === 'Boolean' || (el === 'Function' && options)
: true
})

@edison1105
Copy link
Member

@RicardoErii
This logic is incorrect. No filtering is required if there is a Boolean or Function.

} else if (
type.some(
el =>
el === 'Boolean' ||
((!hasStaticDefaults || defaultString) && el === 'Function')
)
) {
// #4783 for boolean, should keep the type
// #7111 for function, if default value exists or it's not static, should keep it
// in production
return `${finalKey}: { ${concatStrings([
`type: ${toRuntimeTypeString(type)}`,
defaultString
])} }`
} else {

@bgondy
Copy link

bgondy commented Mar 6, 2024

Any update on this issue ?

I've created a reproduction link (library mode) : https://stackblitz.com/edit/vitejs-vite-zz3d8x?file=dist%2Fmy-lib.js&view=editor and I've noticed a weird case.

When typing using TypeScript AND setting type option, the property is duplicated :

<script setup lang="ts">
const a = defineModel<boolean>('a');
const b = defineModel<string>('b');
const c = defineModel<string | boolean>('c');
const d = defineModel<string | boolean>('d', { type: [String, Boolean] });
const e = defineModel('e', {type: [String, Boolean]});
</script>
// lib output
import { defineComponent as t, useModel as o } from 'vue';
const n = /* @__PURE__ */ t({
  __name: 'SampleComponent',
  props: {
    a: { type: Boolean },
    aModifiers: {},
    b: {},
    bModifiers: {},
    c: { type: Boolean }, // <-- Should be {type: [String, Boolean] }
    cModifiers: {},
    d: { type: Boolean, type: [String, Boolean] }, // <-- type property is duplicated with different values
    dModifiers: {},
    e: { type: [String, Boolean] },
    eModifiers: {},
  },
  emits: ['update:a', 'update:b', 'update:c', 'update:d', 'update:e'],
  setup(e) {
    return o(e, 'a'), o(e, 'b'), o(e, 'c'), o(e, 'd'), o(e, 'e'), () => {};
  },
});
export { n as SampleComponent };
//# sourceMappingURL=my-lib.js.map

A possible workaround could be to specify type property, but TypeScript complains about it in my editor :
CleanShot 2024-03-06 at 17 51 46@2x

@sldone
Copy link

sldone commented Apr 8, 2024

Any update on this?

@github-actions github-actions bot locked and limited conversation to collaborators Apr 30, 2024
lynxlangya pushed a commit to lynxlangya/core that referenced this issue May 30, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
🐞 bug Something isn't working scope: compiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants