-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
fix(types): fix generic inference in defineComponent #13770
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
base: main
Are you sure you want to change the base?
fix(types): fix generic inference in defineComponent #13770
Conversation
WalkthroughAdds a new TypeScript overload to defineComponent to infer generic props from setup functions, introduces a runtime-core test suite for generic components, and expands dts tests with generic usage while adjusting one expectation in a runtime-props test. Changes
Sequence Diagram(s)sequenceDiagram
actor Dev
participant TS as TypeScript Type System
participant API as defineComponent (types)
participant Runtime as Component Runtime
Dev->>API: defineComponent(<T>(props: { ... }) => setup, { props, emits, slots })
API->>TS: Infer P from setup(props)
TS-->>API: P (generic props type)
API-->>Dev: DefineSetupFnComponent<P, E, S>
Dev->>Runtime: Mount component with concrete props
Runtime-->>Dev: Rendered output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Unit test describe('defineComponent with generic functions', () => {
test('should preserve type inference for generic functions with props option', () => {
const GenericComp = defineComponent(
<T extends string | number>(props: { value: T; items: T[] }) => {
const count = ref(0)
return () =>
h('div', `${props.value}-${props.items.length}-${count.value}`)
},
{ props: ['value', 'items'] },
)
expect(typeof GenericComp).toBe('object')
expect(GenericComp).toBeDefined()
const root1 = nodeOps.createElement('div')
render(h(GenericComp, { value: 'hello', items: ['world'] }), root1)
expect(serializeInner(root1)).toBe(`<div>hello-1-0</div>`)
const root2 = nodeOps.createElement('div')
render(h(GenericComp, { value: 42, items: [1, 2, 3] }), root2)
expect(serializeInner(root2)).toBe(`<div>42-3-0</div>`)
})
test('should work with complex generic constraints', () => {
interface BaseType {
id: string
name?: string
}
const ComplexGenericComp = defineComponent(
<T extends BaseType>(props: { item: T; list: T[] }) => {
return () => h('div', `${props.item.id}-${props.list.length}`)
},
{ props: ['item', 'list'] },
)
expect(typeof ComplexGenericComp).toBe('object')
const root = nodeOps.createElement('div')
render(
h(ComplexGenericComp, {
item: { id: '1', name: 'test' },
list: [
{ id: '1', name: 'test' },
{ id: '2', name: 'test2' },
],
}),
root,
)
expect(serializeInner(root)).toBe(`<div>1-2</div>`)
})
test('should work with emits option', () => {
const GenericCompWithEmits = defineComponent(
<T extends string | number>(props: { value: T }, { emit }: any) => {
const handleClick = () => {
emit('update', props.value)
}
return () => h('div', { onClick: handleClick }, String(props.value))
},
{
props: ['value'],
emits: ['update'],
},
)
expect(typeof GenericCompWithEmits).toBe('object')
const root = nodeOps.createElement('div')
render(h(GenericCompWithEmits, { value: 'test' }), root)
expect(serializeInner(root)).toBe(`<div>test</div>`)
})
test('should maintain backward compatibility with non-generic functions', () => {
const RegularComp = defineComponent(
(props: { message: string }) => {
return () => h('div', props.message)
},
{ props: ['message'] },
)
expect(typeof RegularComp).toBe('object')
const root = nodeOps.createElement('div')
render(h(RegularComp, { message: 'hello' }), root)
expect(serializeInner(root)).toBe(`<div>hello</div>`)
})
test('should work with union types in generics', () => {
const UnionGenericComp = defineComponent(
<T extends 'small' | 'medium' | 'large'>(props: { size: T }) => {
return () => h('div', props.size)
},
{ props: ['size'] },
)
expect(typeof UnionGenericComp).toBe('object')
const root1 = nodeOps.createElement('div')
render(h(UnionGenericComp, { size: 'small' }), root1)
expect(serializeInner(root1)).toBe(`<div>small</div>`)
const root2 = nodeOps.createElement('div')
render(h(UnionGenericComp, { size: 'large' }), root2)
expect(serializeInner(root2)).toBe(`<div>large</div>`)
})
test('should work with array generics', () => {
const ArrayGenericComp = defineComponent(
<T>(props: { items: T[]; selectedItem: T }) => {
return () => h('div', `${props.items.length}-${props.selectedItem}`)
},
{ props: ['items', 'selectedItem'] },
)
expect(typeof ArrayGenericComp).toBe('object')
const root1 = nodeOps.createElement('div')
render(
h(ArrayGenericComp, {
items: ['a', 'b', 'c'],
selectedItem: 'a',
}),
root1,
)
expect(serializeInner(root1)).toBe(`<div>3-a</div>`)
const root2 = nodeOps.createElement('div')
render(
h(ArrayGenericComp, {
items: [1, 2, 3],
selectedItem: 1,
}),
root2,
)
expect(serializeInner(root2)).toBe(`<div>3-1</div>`)
})
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/runtime-core/src/apiDefineComponent.ts (2)
182-200
: Tighten thesetup
parameter typing to propagate emits/slots and align return type.Two small refinements will improve developer ergonomics and consistency with existing overloads:
- Give
ctx
the typeSetupContext<E, S>
so users get typedemit
/slots
when they don't annotatectx
.- Restrict the return type to
RenderFunction | Promise<RenderFunction>
like the other direct-setup overloads.Apply this diff within the overload:
-export function defineComponent< - F extends (props: any, ctx?: SetupContext<any, any>) => any, - E extends EmitsOptions = {}, - EE extends string = string, - S extends SlotsType = {}, ->( +export function defineComponent< + E extends EmitsOptions = {}, + EE extends string = string, + S extends SlotsType = {}, + F extends ( + props: any, + ctx?: SetupContext<E, S>, + ) => RenderFunction | Promise<RenderFunction>, +>( setup: F, options?: Pick<ComponentOptions, 'name' | 'inheritAttrs'> & { props?: string[] emits?: E | EE[] slots?: S }, ): F extends (props: infer P, ...args: any[]) => any ? P extends Record<string, any> ? DefineSetupFnComponent<P, E, S> : never : neverThis keeps backward compatibility (generic matching still hinges on the function being generic), while upgrading intellisense on
ctx
and ensuring parity with the other direct setup signatures.
182-200
: Consider adding a parallel generic overload for object-form runtime props.Right now, generic setup + props array is covered. For symmetry, you could also support generic setup +
ComponentObjectPropsOptions
(the object form). It avoids relying on the non-generic overload (which is stricter and may not match all generic cases), and keeps the mental model straightforward.Proposed overload to add right after the current one (for reference; place it after existing non-generic overloads to avoid shadowing):
export function defineComponent< E extends EmitsOptions = {}, EE extends string = string, S extends SlotsType = {}, F extends ( props: any, ctx?: SetupContext<E, S>, ) => RenderFunction | Promise<RenderFunction>, >( setup: F, options?: Pick<ComponentOptions, 'name' | 'inheritAttrs'> & { props?: ComponentObjectPropsOptions<any> emits?: E | EE[] slots?: S }, ): F extends (props: infer P, ...args: any[]) => any ? P extends Record<string, any> ? DefineSetupFnComponent<P, E, S> : never : neverThis does not tie the object form to
P
(which isn’t in scope for the parameter list), but it still enables the generic setup function to be matched in scenarios where users prefer object-form runtime props.packages/runtime-core/__tests__/defineComponentGeneric.spec.ts (1)
1-151
: Runtime validation is comprehensive and focused.The cases exercise generic preservation, complex constraints, union generics, and array generics. Rendering checks via
serializeInner
are appropriate.Optional: in the emits test, add a handler to verify the path end-to-end (event emissions reach a listener) to increase confidence:
- render(h(GenericCompWithEmits, { value: 'test' }), root) + const onUpdate = vi.fn() + render(h(GenericCompWithEmits, { value: 'test', onUpdate }), root) + // trigger click + ;(root.firstChild as any).props.onClick() + expect(onUpdate).toHaveBeenCalledWith('test')This is purely additive; current assertions are fine for this PR’s scope.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages-private/dts-test/defineComponent.test-d.tsx
(1 hunks)packages/runtime-core/__tests__/defineComponentGeneric.spec.ts
(1 hunks)packages/runtime-core/src/apiDefineComponent.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/runtime-core/__tests__/defineComponentGeneric.spec.ts (3)
packages-private/dts-test/componentTypeExtensions.test-d.tsx (1)
test
(37-39)packages/runtime-core/src/apiDefineComponent.ts (1)
defineComponent
(324-334)packages/runtime-test/src/serialize.ts (1)
serializeInner
(22-33)
packages/runtime-core/src/apiDefineComponent.ts (5)
packages/runtime-core/src/index.ts (5)
SetupContext
(262-262)EmitsOptions
(292-292)SlotsType
(314-314)ComponentOptions
(276-276)DefineSetupFnComponent
(272-272)packages/runtime-core/src/component.ts (2)
SetupContext
(281-293)ComponentOptions
(276-276)packages/runtime-core/src/componentEmits.ts (1)
EmitsOptions
(41-41)packages/runtime-core/src/componentSlots.ts (1)
SlotsType
(41-43)packages/runtime-core/src/componentOptions.ts (1)
ComponentOptions
(239-294)
packages-private/dts-test/defineComponent.test-d.tsx (1)
packages/runtime-core/src/apiDefineComponent.ts (1)
defineComponent
(324-334)
🔇 Additional comments (2)
packages/runtime-core/src/apiDefineComponent.ts (1)
182-200
: Solid overload to preserve generic props inference (fixes the core issue).This overload bridges the gap for generic setup functions by extracting the props type from the function signature and returning a DefineSetupFnComponent with that type. Well-scoped, placed after the non-generic overloads to avoid shadowing. No runtime impact. Looks good.
packages-private/dts-test/defineComponent.test-d.tsx (1)
1367-1390
: Great coverage for generic function + props array scenario.These assertions validate the intended behavior:
- when using the props array, generic linkage between props is not preserved (mixed types allowed),
- while still preserving stricter generic behavior for the no-options form above.
Nice complement to the new overload.
defineComponent is a Vue.js utility for creating components with TypeScript type inference for TSX, render functions, and IDE support. It accepts a setup function or component options and returns a typed component constructor. With Options: Define components using an options object. Parameters setup: Function taking props and context, returning a render function. Type Parameters Props: Props interface. Examples typescriptimport { defineComponent, h } from 'vue';
const MyComponent = defineComponent({
name: 'MyComponent',
props: { msg: String, count: Number },
setup(props) {
return () => h('div', `${props.msg}: ${props.count}`);
}
}); Generic Component typescriptimport { defineComponent, h } from 'vue';
const GenericComponent = defineComponent(
<T extends string | number>(props: { value: T; list: T[] }) => {
return () => h('div', props.value);
},
{ props: ['value', 'list'] }
); Overload: Setup Function with Props function defineComponent<Props, E, EE extends string, S>(
setup: (props: Props, context: SetupContext<E, EE, S>) => () => VNode,
options?: ComponentOptions
): ComponentConstructor; |
} from '@vue/runtime-test' | ||
import { describe, expect, test } from 'vitest' | ||
|
||
describe('defineComponent with generic functions', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test file is unnecessary, please remove it.
Size ReportBundles
Usages
|
fixes #13763
When using defineComponent with a generic function,
the props type was incorrectly inferred as
any
instead of preserving the generic type, e.g.
{ msg: T; list: T[] }
.Solution
Updated type definitions in defineComponent to
properly preserve generic props inference when
passing a generic function.
Benefits
as any
workaroundsPractical Use Case
Now you can define a generic component:
Summary by CodeRabbit
New Features
Tests