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

feat: Support for generic props component #3682

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

pikax
Copy link
Member

@pikax pikax commented Apr 26, 2021

fix #3102

Allow generic components through a class component declaration.

Class maintain the correct type, if we use function or even the current DefineComponent the finer type will be lost.

This overload allows for the users to override the types to a finer grain

Open for better solutions

example

  type OnChange<ValueType, Clearable> = Clearable extends true
    ? (value: ValueType | null) => void
    : (value: ValueType) => void

  interface GenericProp<Clearable, ValueType> {
    clearable?: Clearable
    value?: ValueType
    onChange?: OnChange<ValueType, Clearable>
  }

  class CompProps<
    Clearable extends boolean,
    ValueType extends string | number | null | undefined
  > extends ComponentPropsOverride<GenericProp<Clearable, ValueType>> {}

  const Comp = defineComponent<typeof CompProps>({
    props: {
      value: Object,
      clearable: Boolean
    }
  })
  ;<Comp
    value={'sss'}
    clearable
    onChange={a => {
      expectType<'sss' | null>(a)
    }}
  />

@pikax pikax requested review from yyx990803 and ktsn April 26, 2021 19:46
PP
> =
// If props is a class we should ifnore all the process
(PropsOrPropOptions extends { prototype: ComponentOptionClass }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the most important part of this PR, this pretty much bypasses our ComponentPublicInstanceConstructor to prevent the Props type to not be modified

@pikax pikax requested a review from HcySunYang April 26, 2021 20:17
PP
> =
// If props is a class we should ifnore all the process
(PropsOrPropOptions extends { prototype: ComponentPropsOverride }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo :)

@HcySunYang
Copy link
Member

Hey @pikax , you have done a great job 👍, it looks simpler than expected, do we need to add some dts test cases for mixins/extends?

@pikax
Copy link
Member Author

pikax commented Apr 27, 2021

do we need to add some dts test cases for mixins/extends?

Not sure, because it will ignore all the mixins/extends the reason is, as soon as we extract the $props: insfer P we loose the Clearable and ValueType generics:

playground

declare function expectType<T>(a: T): void;

type OnChange<ValueType, Clearable> = Clearable extends true
    ? (value: ValueType | null) => void
    : (value: ValueType) => void
interface GenericProp<Clearable, ValueType> {
    clearable?: Clearable
    value?: ValueType
    onChange?: OnChange<ValueType, Clearable>
}


declare function S<
    Clearable extends boolean,
    ValueType extends string | number | null | undefined
>(): {
    $props: GenericProp<Clearable, ValueType>
}

S().$props // $props is not `GenericProp<Clearable, ValueType>`

class C<
    Clearable extends boolean,
    ValueType extends string | number | null | undefined
    > {
    $props = {} as GenericProp<Clearable, ValueType>
}

new C().$props // $props is not `GenericProp<Clearable, ValueType>`

// but `C` has the correct type


// This would also work
type VueProps<T extends new () => { $props: any }> = T
declare const Comp: VueProps<{
    new <
        Clearable extends boolean,
        ValueType extends string | number | null | undefined
        >(): { $props: GenericProp<Clearable, ValueType> }
}>

The templated constructor intact to keep the correct GenericProp<Clearable, ValueType>, which is lost when you construct or extract the $props

I would say this is an advanced usage, with the current implementation it won't support props declared on mixins/extends since to merge those you will need to extract $props which will lose the generic template.

Public API typing would also been lost, since the constructor will only return {$props}, I think the only way is to support class API component (which I started with), but with just setup method (to keep it simple), since the supporting options in Class we already have vue-class-component.

@HcySunYang
Copy link
Member

I would say this is an advanced usage, with the current implementation it won't support props declared on mixins/extends since to merge those you will need to extract $props which will lose the generic template.

I agree with this point

@07akioni
Copy link
Contributor

07akioni commented Jun 2, 2021

Must we create a new class for props definition?

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

Successfully merging this pull request may close these issues.

Is there any way to use generic when defining props?
4 participants