feat(message): add Message component type definition and update the type annotation of the installation method #3252
Conversation
…ype annotation of the installation method
WalkthroughThis change introduces two new TypeScript interfaces in the Vue message module. The Changes
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
WARN GET http://10.0.0.28:4873/@antfu%2Feslint-config error (503). Will retry in 10 seconds. 2 retries left. This error happened while installing a direct dependency of /tmp/eslint ✨ 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:
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 (
|
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
![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 (3)
packages/vue/src/message/index.ts (3)
7-13: Consider consistency in type definition.The
MessageComponentinterface looks good overall and properly extendsComponentPublicInstance. However, there's an inconsistency between how theinstallmethod is typed in the interface (withany) versus its implementation (withVueConstructor).interface MessageComponent extends ComponentPublicInstance { name: string version: string - install: (Vue: any) => void + install: (Vue: VueConstructor) => void componentName: string }
15-18: Consider using a more specific component type.The
VueConstructorinterface is defined minimally for the current usage. However, the component parameter is typed asany, which reduces type safety.interface VueConstructor { - component: (name: string, component: any) => void + component: (name: string, component: ComponentPublicInstance) => void }Additionally, if this is a global type that might be used elsewhere in the codebase, consider moving it to a common types file for reusability.
32-36: Consider adding type checking for window.Vue.The code that installs the component for global Vue instances doesn't have type checking for
window.Vue. Consider adding type assertions for consistency with the rest of the changes.if (process.env.BUILD_TARGET === 'runtime') { if (typeof window !== 'undefined' && window.Vue) { - Message.install(window.Vue) + Message.install(window.Vue as VueConstructor) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/vue/src/message/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
packages/vue/src/message/index.ts (1)
packages/vue/src/modal/index.ts (1)
Modal(81-126)
🔇 Additional comments (3)
packages/vue/src/message/index.ts (3)
5-5: Good addition of type import.Adding the import for
ComponentPublicInstancefrom '@opentiny/vue-common' is appropriate as it supports the type definition of the Message component.
22-22: Good use of type assertion.The type assertion
as MessageComponentcorrectly specifies the type of the Message constant, improving type safety and IDE support.
25-25: Improved type safety for install method.Updating the parameter type from generic
VuetoVueConstructorenhances type safety by explicitly defining what properties are expected on the Vue object.
添加 Message 组件类型定义并更新安装方法的类型注解
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit