fix(types): make generics with runtime props in defineComponent work (fix #11374) #13119
Conversation
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
danyadev
force-pushed
the
fix-generic-defineComponent-with-runtime-props
branch
from
cf9276c to
d7d0e0e
Compare
WalkthroughThe changes enhance the typing system for the Changes
Sequence Diagram(s)sequenceDiagram
participant TSXUser as TSX User
participant CompDef as defineComponent
participant TypeScript as TypeScript Checker
TSXUser->>CompDef: Define component with generics and runtime props
CompDef->>TypeScript: Infer prop types and generics
TypeScript-->>CompDef: Validate types and report errors
TSXUser->>CompDef: Use component in JSX with props and generics
CompDef->>TypeScript: Type-check prop usage
TypeScript-->>TSXUser: Report correctness or errors
Assessment against linked issues
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🪛 Biome (1.9.4)packages/runtime-core/src/apiDefineComponent.ts[error] 167-167: Don't use '{}' as a type. Prefer explicitly define the object shape. '{}' means "any non-nullable value". (lint/complexity/noBannedTypes) [error] 169-169: Don't use '{}' as a type. Prefer explicitly define the object shape. '{}' means "any non-nullable value". (lint/complexity/noBannedTypes) 🔇 Additional comments (3)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
danyadev
changed the title
danyadev
force-pushed
the
fix-generic-defineComponent-with-runtime-props
branch
from
d7d0e0e to
5e1f00a
Compare
|
Hi @jh-leong! Rebased the MR on the main branch to pick up all the changes from the released version, upd: it updated the builds, it seems like the pipelines were a bit clogged at the time and didn't show that they were in progress Also updated the description and hopefully made it fresher and easier to understand, plus added the link to the playground! Really need this feature, so I've been using it right from the creation of this MR :) |
| @@ -157,7 +157,7 @@ export function defineComponent< | |||
| ctx: SetupContext<E, S>, | |||
| ) => RenderFunction | Promise<RenderFunction>, | |||
| options?: Pick<ComponentOptions, 'name' | 'inheritAttrs'> & { | |||
| props?: (keyof Props)[] | |||
| props?: (keyof NoInfer<Props>)[] | |||
There was a problem hiding this comment.
One concern, to avoid breaking this case:
defineComponent(
props => {
// Before: { msg: any }
// After : Record<string, any>
// @ts-expect-error: should error when accessing undefined props
props.foo
// auto-completion missing for props
props.msg
return () => {}
},
{
props: ['msg']
}
)We might want to keep the original behavior by adding a new overload instead:
// overload 1: direct setup function
// (uses user defined props interface)
export function defineComponent<
Props extends Record<string, any>,
E extends EmitsOptions = {},
EE extends string = string,
S extends SlotsType = {},
>(
setup: (
props: Props,
ctx: SetupContext<E, S>,
) => RenderFunction | Promise<RenderFunction>,
options?: Pick<ComponentOptions, 'name' | 'inheritAttrs'> & {
props?: (keyof Props)[]
emits?: E | EE[]
slots?: S
},
): DefineSetupFnComponent<Props, E, S>
+export function defineComponent<
+ Props extends Record<string, any>,
+ E extends EmitsOptions = {},
+ EE extends string = string,
+ S extends SlotsType = {},
+>(
+ setup: (
+ props: Props,
+ ctx: SetupContext<E, S>,
+ ) => RenderFunction | Promise<RenderFunction>,
+ options?: Pick<ComponentOptions, 'name' | 'inheritAttrs'> & {
+ props?: (keyof NoInfer<Props>)[]
+ emits?: E | EE[]
+ slots?: S
+ },
+)There was a problem hiding this comment.
I see, but the proposed solution breaks another thing: if runtime props contain more props than declared in props, and the function is generic, the types fall back to any:
(@ts-expect-error is red because there was an error, but it's gone now)
It's still the best solution though, I've tried some variants and none of them worked
There was a problem hiding this comment.
applied your solution for now and added a few tests
There was a problem hiding this comment.
I believe this issue is negligible in practice — it only happens when props is both generic and doesn't match the runtime props, which seems rare.
jh-leong
added
the
🍰 p2-nice-to-have
danyadev
force-pushed
the
fix-generic-defineComponent-with-runtime-props
branch
from
5e1f00a to
b8cc94b
Compare
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
fixes #11374
relates to #12761 (comment), #7963 (comment)
Problem
The "support" for generics in defineComponent was introduced in #7963, but it simply doesn't work: when you pass the
propsoption, TypeScript ignores the main props definition and infers the props type from thepropsoption instead.Unfortunately, this option doesn't provide any useful type hints except for the names of the props, so all the props become
anyHere is a simple example, where instead of expected normal types we encounter
any:https://play.vuejs.org/#eNp9Ustu2zAQ/JUFL1IAQ0bR9qLKAtogh/bQBIlvYRAY0lphQi8FklIcCPr3LCnLeSIniTO7s7OPQfxu26zvUOSicJVVrQe9oWYlhXd7KUpJatca62GAGreK8NTwm5A8jLC1ZgcJZye/JEnyTy3ChTWtK9YlrGCQBOBQY+WxzmEdnoZO71gfc0hN65Uhxk9gVUJvVC1pDDqVIefhKiayzLu6abEG3Huk2oHzVlFTpm0omh9rR8FY3aLvLEEakaJWffniZ4hZ2QyMxTLw7GEx5R5Er5M5IllAMvtPbjjwJLjFfZwPu9x0On7fuJ1KfzSTBgQmT9MvPw49zwV5C1tjpDhObTWkMxdFwqSMxkyb5oUYYXlQnDsCYKfBbbGcdsyYWPCGOX+rmuzeGeIDiCalqNi70mjP436cFDyqSU+Kjdbm8V/EvO1wMePVHVYPn+D34Yhy/rmw6ND2KMWR8xvboJ/os6v/vNNX5M7UneboL8hL5N674HEK+9NRzbZfxUW3f+P98pms3Vk4Gzc3FYyGSL65kadx26MNHA/ie/Yz+/ZDjM+7YwkP
Solution
Let's look at one of the overloads of defineComponent:
Here we can see the
Propsgeneric type, thesetupargument usingPropsand theoptionsargument also usingPropsWhen we add a generic type to the
setupfunction, it becomes less obvious for TypeScript to decide which variable to use to infer the generic type, and unfortunately for us it chooses the variant with less type density.So we need to tell TypeScript not to infer
Propsfrom theoptions.propsfield:Another solution
Initially I've come up with another solution which doesn't rely on that new TypeScript
NoInfertype. But as you already useNoInferin runtime code, it may be irrelevant.It works by separating
Propsinto two generics — originalPropsandDeclaredProps— and using them differently:A note about an object format for props with runtime validations
This MR fixes only the usage with props defined as an array of strings. I haven't found any solution for the object format and I'm not sure that there is one...
But on the other side, I don't think someone would need to combine generics with runtime validations
Summary by CodeRabbit
Summary by CodeRabbit
Tests
New Features