fix(types): avoid merging component instance into $props in ComponentInstance
#12870
Conversation
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/runtime-core
@vue/reactivity
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
edison1105
added
ready to merge
WalkthroughThis change refines type inference for component instances by updating the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant TypeSystem as TypeScript Type System
participant Component as Component (from defineComponent)
Dev->>TypeSystem: Instantiate ComponentInstance<typeof Component>
TypeSystem->>Component: Check if type has $props property
alt Has $props
TypeSystem-->>Dev: Return type as-is (no nested $props)
else No $props
TypeSystem-->>Dev: Construct ComponentPublicInstance type
end
Assessment against linked issues
Poem
✨ Finishing Touches
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/runtime-core/src/component.ts (1)
129-131: Consider using specific object types instead of empty objectsWhile the use of
{}as fallback types when the original types areunknownworks functionally, it's generally better to use more specific object types.Static analysis tools flag empty object types because
{}represents "any non-nullable value" rather than an empty object. Consider using more specific types or useRecord<string, never>for a truly empty object type if that's what's intended.🧰 Tools
🪛 Biome (1.9.4)
[error] 129-129: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 130-130: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 131-131: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
packages-private/dts-test/componentInstance.test-d.tsx(1 hunks)packages/runtime-core/src/component.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages-private/dts-test/componentInstance.test-d.tsx (1)
packages/runtime-core/src/component.ts (1)
ComponentInstance(113-135)
🪛 Biome (1.9.4)
packages/runtime-core/src/component.ts
[error] 129-129: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 130-130: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 131-131: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 270-270: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🔇 Additional comments (3)
packages/runtime-core/src/component.ts (2)
118-134: Improved type inference for component instances returned by defineComponent()This change correctly refines the
ComponentInstance<T>type alias to handle components returned bydefineComponent(). By renaming the parameter fromPropstoPropsOrInstanceand adding a conditional type check, the type system now correctly differentiates between raw props and component instances.The conditional check
PropsOrInstance extends { $props: unknown }is an elegant solution that avoids double-wrapping the component instance inComponentPublicInstancewhen it already has a$propsproperty.🧰 Tools
🪛 Biome (1.9.4)
[error] 129-129: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 130-130: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 131-131: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
265-274: Properly propagated type parameter rename to Component interfaceThe parameter rename from
PropstoPropsOrInstanceis correctly propagated throughout the related type definitions, ensuring consistency across the type system.🧰 Tools
🪛 Biome (1.9.4)
[error] 270-270: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
packages-private/dts-test/componentInstance.test-d.tsx (1)
141-154: Great test coverage for type correction in $propsThis test case correctly verifies the fix for issue #12751 by ensuring:
- Component emits are properly typed in the component instance
- Event handlers are correctly typed in the
$propsobject- No nested
$propsproperty exists within the$propsobject itselfThe test case is well-structured and properly uses the TypeScript
@ts-expect-errorannotation to verify that the fix prevents the nesting of$propswithin itself.
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
close #12751
Previously,
ComponentInstanceincorrectly merged the component instance into$props, causing$propsto contain an extra$xxxfield.This fix ensures that when
defineComponent()is used,ComponentInstancedirectly resolves to the correct instance type instead of wrapping it inComponentPublicInstance.Summary by CodeRabbit
Tests
Refactor