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

Allow ExtractPropTypes to make props with default values optional #3122

Open
kiroushi opened this issue Jan 29, 2021 · 5 comments
Open

Allow ExtractPropTypes to make props with default values optional #3122

kiroushi opened this issue Jan 29, 2021 · 5 comments

Comments

@kiroushi
Copy link

kiroushi commented Jan 29, 2021

Version

3.0.5

Reproduction link

https://codesandbox.io/s/stoic-hamilton-t8cft?file=/src/index.ts

Steps to reproduce

Access props via setup() block or use ExtractPropTypes utility to check props type inference (reproduction link has a naive example).

image

What is expected?

Props with a default property configured shouldn’t be marked as required by TS.

Edit: Seems like maybe I misunderstood the purpose of said utility. ExtractPropTypes is giving type safety to props argument, but not for objects that are safe to be bound to components with such properties.

Maybe there should be another utility that yield proper types from the binding perspective (as in, marking props with default properties as optional).

What is actually happening?

RequiredKeys<> TS utility is marking props that have default property as required (even if they are explicitly configured with required: false).

@jesusgn90
Copy link

+1

@HcySunYang
Copy link
Member

You really misunderstood the purpose of ExtractPropTypes, but I think it is valuable if ExtractPropTypes is allowed to make props with default values optional.

@HcySunYang HcySunYang changed the title RequiredKeys is marking props with defaults as required Allow ExtractPropTypes to make props with default values optional Jan 30, 2021
@07akioni
Copy link
Contributor

07akioni commented Feb 1, 2021

Update: the comment is outdated. HcySunYang's ExtractPropsType has parameter on whether to make default props optional.

For props will be passed to h, you can turn it on.
For props from setup, you can keep the default parameter.


You really misunderstood the purpose of ExtractPropTypes, but I think it is valuable if ExtractPropTypes is allowed to make props with default values optional.

It is needed to distinguish props which is passed from outside and the props can be used inner the component.

For

{
  props: {
    foo: {
       type: String
       default: '123'
    }
}

The type of passed props from its parent should be { foo?: string } but the props received from its own setup should be { foo: string }.

Since ExtractPropTypes is used to generate setup's prop value. I think we can't simply make it optional.
image

@kiroushi
Copy link
Author

kiroushi commented Feb 2, 2021

Yeah, it immediately clicked as soon as I checked CI outcome. Sorry for the hassle.

Would it make sense to expose a prop utility to get typings from a parent’s scope perspective, like HcySunYang mentioned?

@zouyaoji
Copy link

zouyaoji commented Jan 12, 2022

+1

const todoItemProps = {
  item: {
    type: String,
    default: '',
    required: false
  },
  complete: {
    type: Boolean,
    default: false,
    required: false
  }
}
export type TodoItemProps = ExtractPropTypes<typeof todoItemProps>
const myDodoItemProps: TodoItemProps = {
  item: 'd'
}

throws an exception:
录制_2022_01_12_23_56_28_918

As shown in the figure, when the required parameter of props is false, the declaration should be an optional attribute.
如图,props的required参数为false时,声明应该是可选属性才正确吧。

But it is actually prompted that there is still a complete property that is not defined, so I suspect that there may be a problem with ExtractPropTypes?
但实际上确提示说还有一个 complete 属性未定义,由此怀疑 ExtractPropTypes 可能有问题?

Is this an unresolved issue, or am I not using it correctly?
请问这是个还没解决的issue吗,还是我没有正确使用?

@HcySunYang

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

Successfully merging a pull request may close this issue.

5 participants