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

Problem typing optional props with exactOptionalPropertyTypes enabled in tsconfig #6532

Open
2-5 opened this issue Aug 22, 2022 · 15 comments
Open

Comments

@2-5
Copy link

2-5 commented Aug 22, 2022

Vue version

3.2.37

Link to minimal reproduction

https://sfc.vuejs.org/#eNqdUzuP1DAQ/itWqpx0iREnhBSyuaUAiQaoqNxkk9nFt/FDfgShkP/OOE72QnK6gs6eGX+PmfGQfNQ67z0kRVI6ELqrHVRMElJ+kdq7r16cwJA+E6qF7sCSvu48sITQWKSrYSBTjDw+EpZ42cIZ0+NYUh1LTt45Jcmx6XhzXRDIgbx7w5Lq27WksQCLS7pSgFfbGK4dseC8Jl0tL/jaWXzFJBdaGUcGYuBMRnI2SiA7+mDJLbk2MBfkdBUMtkM5k42S1s02DgGylPHZHzIZ4hLaKr0d7z4ErVEdaknuk8iYiVrnT1ZJbOYQvLM5gZoLMkVCbKIt8PDTOW0LSr3U10veKEGPmKPGS8cFZK0Sx4f8bf7wnrbcunU8Byuyk1G/LNp4Qvj7FTjFYA8mM4CCDZhXyTa1/xBucjvSwDkyOWIDNm3dbxMPBas9arxBXPdjXif3WwNGY9vn9fqfdUBfyAPtbieWIWujtMUhx1F+D7dymswkbJJTkP300WWVhrkvOCC4u8F8wssMk+Jz/Aa6RdnFMyZOKO7XS+B3BekV33Ose4Rci7d0IrqAS/HhHm1ZNIN9MjIazp+VID5ObdoX7GS6FWWd4fLygroFdvklujYWPneqnjGC8JAPjUlf60AsDArG7Uca/wK66Je8

Steps to reproduce

See reproduction link for full example.

I declare a property which allows undefined:

const props = defineProps<{
  modelValue: number | undefined
}>();

and actually pass undefined:

<template>
  <TheComponent v-model="value" />
</template>

<script setup lang="ts">
const value = ref<number | undefined>(undefined);
</script>

What is expected?

Vue doesn't emit runtime errors for explicitly allowed and used undefined properties.

What is actually happening?

I get the following Vue runtime error in dev mode

[Vue warn]: Invalid prop: type check failed for prop "modelValue". Expected Number | Null, got Undefined  

This is because the SFC compiler emits the following property declaration. Note how null was used instead of undefined:

props: {
  modelValue: { type: [Number, null], required: true }
},

If I modify the generated code and runtime like below (see <<< and >>>), to allow undefined, it seems to work correctly. But I don't understand if there are other reasons for not allowing undefined to be a required property value:

// Component
props: {
  modelValue: { type: [Number, undefined], required: true }
},

// Vue Runtime
function getType(ctor) {
  const match = ctor && ctor.toString().match(/^\s*function (\w+)/);
  // <<<<<<<< added `undefined` support >>>>>>>>>>
  return match ? match[1] : ctor === null ? "null" : ctor === undefined ? "undefined" : "";
}

function assertType(value, type) {
  let valid;
  const expectedType = getType(type);
  if (isSimpleType(expectedType)) {
    const t = typeof value;
    valid = t === expectedType.toLowerCase();
    if (!valid && t === "object") {
      valid = value instanceof type;
    }
  } else if (expectedType === "Object") {
    valid = isObject(value);
  } else if (expectedType === "Array") {
    valid = isArray$1(value);
  } else if (expectedType === "null") {
    valid = value === null;
  // <<<<<<<< added `undefined` support >>>>>>>>>>
  } else if (expectedType === "undefined") {
    valid = value === undefined;
  } else {
    valid = value instanceof type;
  }
  return {
    valid,
    expectedType
  };
}

System Info

System:
  OS: Windows
Binaries:
  Node: 18.7.0
  npm: 8.15.0
npmPackages:
  vue: ^3.2.37 => 3.2.37

Any additional comments?

I'm using undefined when a value is not set by the user, for example empty number <input> or unselected <select>, as I understood to be current recommended practices:

Use undefined. Do not use null.
https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines#null-and-undefined

@LinusBorg
Copy link
Member

For now, just use ?: in the prop type to make the prop optional. Also allows for rhe prop to be undefined.

const props = defineProps<{
  modelValue?: number
}>();

@2-5
Copy link
Author

2-5 commented Aug 23, 2022

That's what I originally tried, but I also have "exactOptionalPropertyTypes": true in my tsconfig.json (https://devblogs.microsoft.com/typescript/announcing-typescript-4-4-beta/#exact-optional-property-types) and I get vue-tsc errors:

Type '{ modelValue: number | undefined; }' is not assignable to type 
'Omit<Readonly<ExtractPropTypes<__VLS_TypePropsToRuntimeProps<{ modelValue?: number; }>>>
& { "onUpdate:modelValue"?: (value: number | undefined) => any; }
& VNodeProps 
& AllowedComponentProps
& ComponentCustomProps, never>' with 'exactOptionalPropertyTypes: true'.
Consider adding 'undefined' to the types of the target's properties. ts(2375)

If I add | undefined, it still doesn't work:

const props = defineProps<{
  modelValue?: number | undefined
}>();
Type '{ modelValue: number | undefined; }' is not assignable to type 
'Omit<Readonly<ExtractPropTypes<__VLS_TypePropsToRuntimeProps<{ modelValue?: number | undefined; }>>>
& { "onUpdate:modelValue"?: (value: number | undefined) => any; }
& VNodeProps
& AllowedComponentProps
& ComponentCustomProps, never>' with 'exactOptionalPropertyTypes: true'.
Consider adding 'undefined' to the types of the target's properties. ts(2375)

This could be a vue-tsc bug, I'm not sure.

@LinusBorg
Copy link
Member

LinusBorg commented Aug 25, 2022

When and where exactly is that error being raised? Do you also get an error in VSCode/your IDE?

As an experiment, try this please:

const props = defineProps<{
  modelValue?: number | undefined
  'onUpdate:modelValue'?: undefined | (value: number | undefined) => any
}>

and don't use defineEmits()

I'd suspect that vue(-tsc) has a problem with the event prop generated from the defineEmits rather than the modelValue prop, as that one is missing the undefined in the ts error message.

@2-5
Copy link
Author

2-5 commented Aug 25, 2022

I get the errors both in VS Code with Volar when the file is open and when running npx vue-tsc -noEmit in the terminal in a vite based project.

I tried your changes, I still get type errors.

Volar:

Type '{ modelValue: number | undefined; }' is not assignable to type 
'Omit<Readonly<ExtractPropTypes<__VLS_TypePropsToRuntimeProps<{ modelValue?: number | undefined; "onUpdate:modelValue"?: ((value: number | undefined) => any) | undefined; }>>>
& VNodeProps
& AllowedComponentProps
& ComponentCustomProps, never>' with 'exactOptionalPropertyTypes: true'.
Consider adding 'undefined' to the types of the target's properties. ts(2375)

npx vue-tsc -noEmit:

error TS2375: Type '{ modelValue: number | undefined; }' is not assignable to type
'IntrinsicAttributes
& Partial<{}>
& Omit<Readonly<ExtractPropTypes<__VLS_TypePropsToRuntimeProps<{ modelValue?: number | undefined; "onUpdate:modelValue"?: ((value: number | undefined) => any) | undefined; }>>>
& VNodeProps
& AllowedComponentProps
& ComponentCustomProps, never>' with 'exactOptionalPropertyTypes: true'.
Consider adding 'undefined' to the types of the target's properties.
  Type '{ modelValue: number | undefined; }' is not assignable to type 
'Omit<Readonly<ExtractPropTypes<__VLS_TypePropsToRuntimeProps<{ modelValue?: number | undefined; "onUpdate:modelValue"?: ((value: number | undefined) => any) | undefined; }>>>
& VNodeProps
& AllowedComponentProps
& ComponentCustomProps, never>' with 'exactOptionalPropertyTypes: true'.
Consider adding 'undefined' to the types of the target's properties.
    Types of property 'modelValue' are incompatible.
      Type 'number | undefined' is not assignable to type 'number'.
        Type 'undefined' is not assignable to type 'number'.

@2-5
Copy link
Author

2-5 commented Aug 25, 2022

The problem also appears with simple properties, so it's not modelValue specific:

2022-08-25_142525

@LinusBorg
Copy link
Member

LinusBorg commented Aug 25, 2022

Okay, so I can kinda reproduce this, but a full repro from your side in a github repo would be apprechiated to make sure we are looking at the same things.

I have:

  • exactOptionalPropertyTypes: true in my tsconfig
  • Typescript 4.6 in my project i'm testing with. (yes, not latest)
  • latest Volar / vue-tsc (0.40.1)

This is HelloWorld.vue ...:

const props = defineProps<{
  msg?: string | undefined;
}>();

This is what I have in the parent's state:

import { ref, Ref } from 'vue'

const nameRef: Ref<string | undefined> = ref("Tom");
let name: string | undefined = "Tom";

Both, when used for msg, will throw the error.

image

I think it is because the type generated for the props object that will be passed to the child is:

{
  msg: string | undefined
}

while the type from the child component is:

{
  msg?: string | undefined
}

So we could get rid of the question mark, but then we can't leave the prop completely absent in the parent.

Not sure if this needs to be solved in Vue core, our JSX types or vue-tsc?

/cc @pikax @johnsoncodehk What do you think?

@LinusBorg LinusBorg changed the title Problem with allowing property to be undefined Problem typing optional props with exactOptionalPropertyTypes enabled in tsconfig Aug 25, 2022
@2-5
Copy link
Author

2-5 commented Aug 25, 2022

In plain TypeScript there is no problem with leaving the question mark in, so it is related to Vue & co somehow. There are no reported errors in the code below with exactOptionalPropertyTypes: true

// test.ts

interface Props {
  required: string
  optional?: string | undefined
}

const p1: Props = { required: "req", optional: "opt" }
const p2: Props = { required: "req", optional: undefined }
const p3: Props = { required: "req" }

let optional: string | undefined = "opt"
const p4: Props = { required: "req", optional }

export { p1, p2, p3, p4 }

@LinusBorg
Copy link
Member

LinusBorg commented Aug 25, 2022

That's because in your example you are essentially casting all 4 objects to be of type Prop. They don't have their own explicit type that could clash with your : Props annotation.

I think what happens in this issue is more like this (pseudocode, and a bit backwards):

interface PropsExtractedFromParentTemplate {
  msg?: string | undefined
}

interface PropsFromChildComponentDef {
  msg: string | undefined
}

const propsFromParent: PropsExtractedFromParentTemplate  = {
  msg: 'Tom'
}

propsFromParent.msg

function ChildComponent(props: PropsFromChildComponentDef) {
  props.msg
}

ChildComponent(propsFromParent) // will error

TS Playground

@pikax
Copy link
Member

pikax commented Aug 25, 2022

I would say having | undefined or ?: on a prop should generate basically the same code, because in vue if a prop is declare it will be always available.

I think this problem is related with the generation of the code when using script setup and Volar.

@LinusBorg
Copy link
Member

LinusBorg commented Aug 30, 2022

@pikax So there's two layers to this:

  1. Runtime/codegen: There's the problem of code generation: an optional?: prop generates a proper runtime prop definition, while a prop with x | undefined doesn't. The PR that was already submitted seems to fix this.
  2. Types: the types that vue-tsc/Volar generates from the defineProps generic argument are not practical when exactOptionalPropertyTypes is being used. This has to be solved in Volar.

Would you agree?

johnsoncodehk added a commit to vuejs/language-tools that referenced this issue Aug 30, 2022
@johnsoncodehk
Copy link
Member

For Volar/vue-tsc, please track vuejs/language-tools#1798.

@fabon-f
Copy link

fabon-f commented Oct 30, 2022

There are one more problem: Type definitions of native elements.

For example, this simple component from Nuxt Content has type error with exactOptionalPropertyTypes option, because in this component width can be undefined while native img's width prop has optional non-undefined Numberish type. (under this TS option, optional and allowing undefined is different)

https://github.com/nuxt/content/blob/409f03f81f3023d314a7a4d995dec6a726d79a60/src/runtime/components/Prose/ProseImg.vue

This problem can't be resolved without modifying native types in packages/runtime-dom/types/jsx.d.ts, so I suggest adding | undefined to all optional props.

ref: #6068

For your reference, in the type definition of react, all optional props has | undefined to support exactOptionalPropertyTypes.
DefinitelyTyped/DefinitelyTyped#54352

@2-5
Copy link
Author

2-5 commented Oct 30, 2022

The linked PR (#6533) does solve the issue for me.

Maybe it can be looked at and merged in?

@atlansien
Copy link

Any progress on this issue?

@YasserB94
Copy link

Bump - I also have this issue


Here's what works for me now

<script setup lang="ts">
     import {ButtonHTMLAttributes} from 'vue';
     defineProps<ButtonHTMLAttributes>();
</script>
<template>
	<button :type="type ?? 'button'">
		<slot />
	</button>
</template>

Here's what I expect to work

<script setup lang="ts">
     import {ButtonHTMLAttributes} from 'vue';

     withDefaults(defineProps<ButtonHTMLAttributes>(), {type: 'button'});
</script>
<template>
	<button :type="type" >
		<slot />
	</button>
</template>

Error caused

error TS2379: Argument of type '{ type: "button" | "reset" | "submit" | undefined; class: string; }' is not assignable to parameter of type 'ButtonHTMLAttributes & ReservedProps & Record<string, unknown>' with 'exactOptionalPropertyTypes: true'.

Consider adding 'undefined' to the types of the target's properties.
 Type '{ type: "button" | "reset" | "submit" | undefined; class: string; }' is not assignable to type 'ButtonHTMLAttributes' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
    Types of property 'type' are incompatible.
      Type '"button" | "reset" | "submit" | undefined' is not assignable to type '"button" | "reset" | "submit"'.
        Type 'undefined' is not assignable to type '"button" | "reset" | "submit"'.

Vue ButonHTMLAttributes Interface for reference

export interface ButtonHTMLAttributes extends HTMLAttributes {
    autofocus?: Booleanish;
    disabled?: Booleanish;
    form?: string;
    formaction?: string;
    formenctype?: string;
    formmethod?: string;
    formnovalidate?: Booleanish;
    formtarget?: string;
    name?: string;
    type?: 'submit' | 'reset' | 'button'; // It feels like adding | undefined here will suppress the error ?
    value?: string | string[] | number;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants