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

defineModel ignores TS types #9587

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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Something isn't working scope: compiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants