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(tsx): add support for passing generics to child components #11478

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions packages-private/dts-test/defineComponent.test-d.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2027,3 +2027,70 @@ expectString(instance.actionText)
// public prop on $props should be optional
// @ts-expect-error
expectString(instance.$props.actionText)

describe('generic components in defineComponent', () => {
const GenericComp = defineComponent(
<T extends { name: string }>(props: { msg: string; list: T[] }) => {
return () => (
<div>
{props.msg}
{props.list.map(item => item.name).join(', ')}
</div>
)
},
)

const GenericCompUser = defineComponent(() => {
const list = ref([{ name: 'Tom' }, { name: 'Jack' }])

return () => {
return (
<div>
<GenericComp<{ name: string }> msg="hello" list={list.value} />
</div>
)
}
})

// Test correct usage
expectType<JSX.Element>(<GenericCompUser />)

// Test GenericComp directly with correct props
expectType<JSX.Element>(
<GenericComp<{ name: string }> msg="hello" list={[{ name: 'Alice' }]} />,
)

// Test with missing required prop
expectType<JSX.Element>(
// @ts-expect-error
<GenericComp<{ name: string }> list={[{ name: 'Bob' }]} />,
)

// Test with extended type
interface Person {
name: string
age: number
}

const ExtendedGenericCompUser = defineComponent(() => {
const people = ref<Person[]>([
{ name: 'Tom', age: 25 },
{ name: 'Jack', age: 30 },
])

return () => {
return (
<div>
<GenericComp<Person> msg="people" list={people.value} />
</div>
)
}
})

expectType<JSX.Element>(<ExtendedGenericCompUser />)

// Test GenericComp directly with extended type
expectType<JSX.Element>(
<GenericComp<Person> msg="people" list={[{ name: 'Alice', age: 28 }]} />,
)
})
117 changes: 107 additions & 10 deletions packages/runtime-core/src/apiDefineComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@ import type {
TypeEmitsToOptions,
} from './componentEmits'
import { extend, isFunction } from '@vue/shared'
import type { VNodeProps } from './vnode'
import type { VNode, VNodeProps } from './vnode'
import type {
ComponentPublicInstanceConstructor,
CreateComponentPublicInstanceWithMixins,
} from './componentPublicInstance'
import type { SlotsType } from './componentSlots'
import type { Directive } from './directives'
import type { ComponentTypeEmits } from './apiSetupHelpers'
import type { Ref } from '@vue/reactivity'

export type PublicProps = VNodeProps &
AllowedComponentProps &
Expand Down Expand Up @@ -134,6 +135,58 @@ export type DefineSetupFnComponent<
S
>

export type DefineComponentWithGeneric<
PropsOrPropOptions = {},
RawBindings = {},
D = {},
C extends ComputedOptions = ComputedOptions,
M extends MethodOptions = MethodOptions,
Mixin extends ComponentOptionsMixin = ComponentOptionsMixin,
Extends extends ComponentOptionsMixin = ComponentOptionsMixin,
E extends EmitsOptions = {},
EE extends string = string,
PP = PublicProps,
Props = ResolveProps<PropsOrPropOptions, E>,
Defaults = ExtractDefaultPropTypes<PropsOrPropOptions>,
S extends SlotsType = {},
Generic extends Record<string, any> = {},
> = ComponentPublicInstanceConstructor<
CreateComponentPublicInstanceWithMixins<
Props,
RawBindings,
D,
C,
M,
Mixin,
Extends,
E,
PP & Props,
Defaults,
true,
{},
S
> &
Generic
> &
ComponentOptionsBase<
Props,
RawBindings,
D,
C,
M,
Mixin,
Extends,
E,
EE,
Defaults,
{},
string,
S
> &
PP & {
<T extends Generic>(props: Props & { ref?: Ref<any> } & T): VNode
}

// defineComponent is a utility that is primarily used for type inference
// when declaring components. Type inference is provided in the component
// options (provided as the argument). The returned value has artificial types
Expand Down Expand Up @@ -290,16 +343,60 @@ export function defineComponent<
TypeRefs
>

export function defineComponent<
T extends Record<string, any> = {},
PropsOptions extends Readonly<ComponentPropsOptions> = {},
RawBindings = {},
D = {},
C extends ComputedOptions = {},
M extends MethodOptions = {},
Mixin extends ComponentOptionsMixin = ComponentOptionsMixin,
Extends extends ComponentOptionsMixin = ComponentOptionsMixin,
E extends EmitsOptions = {},
EE extends string = string,
S extends SlotsType = {},
>(
options: ComponentOptionsBase<
PropsOptions,
RawBindings,
D,
C,
M,
Mixin,
Extends,
E,
EE,
ExtractDefaultPropTypes<PropsOptions>,
{},
string,
S
> & { __generic?: T },
): DefineComponentWithGeneric<
PropsOptions,
RawBindings,
D,
C,
M,
Mixin,
Extends,
E,
EE,
PublicProps,
Readonly<PropsOptions>,
ExtractDefaultPropTypes<PropsOptions>,
S,
T
>

// implementation, close to no-op
/*! #__NO_SIDE_EFFECTS__ */
export function defineComponent(
options: unknown,
extraOptions?: ComponentOptions,
) {
return isFunction(options)
? // #8326: extend call and options.name access are considered side-effects
// by Rollup, so we have to wrap it in a pure-annotated IIFE.
/*#__PURE__*/ (() =>
extend({ name: options.name }, extraOptions, { setup: options }))()
export function defineComponent(options: unknown): any {
const comp = isFunction(options)
? { setup: options, name: options.name }
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason for this change? It doesn't seem to be related to types.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@edison1105
The original code had logic to convert function components into objects, but I changed the runtime behavior using Proxy. This was intended to handle props when using Generic Components in TSX.

As you pointed out, this isn't related to types. I realized that by removing extraOptions in this modification, I might have eliminated the ability to extend types.

I'm planning to revert these changes, but is my thought process correct?

Thank you for reviewing multiple times.

Copy link
Member

Choose a reason for hiding this comment

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

This was intended to handle props when using Generic Components in TSX.

  • I am not sure about this, keep this, waiting for other team members' review

  • It looks like the test case is incorrect because it still passes without the changes in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for review!

It looks like the test case is incorrect because it still passes without the changes in this PR.

Does the fact that the test passes without this change mean that the following writing style already exists?

// Test GenericComp directly with correct props
expectType<JSX.Element>(
<GenericComp<{ name: string }> msg="hello" list={[{ name: 'Alice' }]} />,
)

Or is there a possibility that the original type checking was loose and designed not to throw errors regardless of the writing style?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@edison1105
It has been left here for a long time in a state where I can't proceed with anything. Is there anything I can do?

: options
return new Proxy(comp as any, {
apply(target, thisArg, argumentsList) {
return extend({}, target, { props: argumentsList[0] })
},
})
}
1 change: 1 addition & 0 deletions packages/runtime-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ export type {
export type {
DefineComponent,
DefineSetupFnComponent,
DefineComponentWithGeneric,
PublicProps,
} from './apiDefineComponent'
export type {
Expand Down
2 changes: 1 addition & 1 deletion packages/runtime-dom/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {
type App,
type CreateAppFunction,
type DefineComponent,
type DefineComponentWithGeneric as DefineComponent,
DeprecationTypes,
type Directive,
type ElementNamespace,
Expand Down