-
Notifications
You must be signed in to change notification settings - Fork 7.8k
fix: 优化组件方法透传并新增表单弹窗示例 #6443
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
fix: 优化组件方法透传并新增表单弹窗示例 #6443
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the """ WalkthroughThe code refactors the internal implementation of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BasicFormDemo
participant ModalDemo
participant FormApi
User->>BasicFormDemo: Click "打开弹窗" button
BasicFormDemo->>ModalDemo: modalApi.open()
ModalDemo->>FormApi: onOpenChange (populate form if values exist)
User->>ModalDemo: Confirm in modal
ModalDemo->>FormApi: formApi.validateAndSubmitForm()
FormApi-->>ModalDemo: Validation result
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
Poem
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 (
|
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: 1
🧹 Nitpick comments (1)
apps/web-naive/src/views/demos/form/modal.vue (1)
10-45
: Form schema is well-structured with minor improvement needed.The form configuration is comprehensive with proper validation rules. However, consider improving the placeholder text for better UX.
Apply this diff to improve the Select component placeholder:
{ component: 'Select', componentProps: { options: [ { label: '选项1', value: '1' }, { label: '选项2', value: '2' }, ], - placeholder: '请输入', + placeholder: '请选择', }, fieldName: 'field3', label: '字段3', rules: 'required', },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web-antd/src/adapter/component/index.ts
(2 hunks)apps/web-ele/src/adapter/component/index.ts
(2 hunks)apps/web-naive/src/adapter/component/index.ts
(2 hunks)apps/web-naive/src/views/demos/form/basic.vue
(3 hunks)apps/web-naive/src/views/demos/form/modal.vue
(1 hunks)playground/src/adapter/component/index.ts
(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
apps/web-naive/src/views/demos/form/basic.vue (5)
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5397
File: playground/src/bootstrap.ts:23-30
Timestamp: 2025-01-15T04:29:13.944Z
Learning: In the Vue-Vben-Admin project, commented code in bootstrap.ts showing modal and drawer configuration examples should be preserved as reference documentation, even after moving to CSS variables for z-index management.
Learnt from: ArthurDarkstone
PR: vbenjs/vue-vben-admin#4807
File: docs/src/components/common-ui/vben-vxe-table.md:65-84
Timestamp: 2024-11-05T10:34:13.846Z
Learning: In `docs/src/components/common-ui/vben-vxe-table.md`, the code examples are provided within markdown scripts for documentation and do not require import statements or explicit type annotations.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5587
File: playground/src/views/examples/loading/index.vue:15-18
Timestamp: 2025-02-23T04:21:24.691Z
Learning: Chinese text in the description of the loading component example (`playground/src/views/examples/loading/index.vue`) is intentionally kept without i18n support.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5013
File: docs/src/components/layout-ui/page.md:31-31
Timestamp: 2024-12-04T04:41:30.161Z
Learning: 在 Vben Admin 项目的 `Page` 组件中,`extra` 内容仅通过 slots 提供,没有对应的 prop 属性。
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5013
File: docs/src/components/layout-ui/page.md:23-24
Timestamp: 2024-12-04T04:43:22.179Z
Learning: 在 `docs/src/components/layout-ui/page.md` 的 `Page` 组件中,`title` 和 `description` 属性有意使用 `string|slot` 类型表示法,表示它们可以接受字符串或插槽。
apps/web-ele/src/adapter/component/index.ts (4)
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5397
File: playground/src/bootstrap.ts:23-30
Timestamp: 2025-01-15T04:29:13.944Z
Learning: In the Vue-Vben-Admin project, commented code in bootstrap.ts showing modal and drawer configuration examples should be preserved as reference documentation, even after moving to CSS variables for z-index management.
Learnt from: ArthurDarkstone
PR: vbenjs/vue-vben-admin#4807
File: docs/src/components/common-ui/vben-vxe-table.md:65-84
Timestamp: 2024-11-05T10:34:13.846Z
Learning: In `docs/src/components/common-ui/vben-vxe-table.md`, the code examples are provided within markdown scripts for documentation and do not require import statements or explicit type annotations.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5446
File: packages/effects/common-ui/src/components/icon-picker/icons.ts:0-0
Timestamp: 2025-01-21T05:03:44.466Z
Learning: In the vue-vben-admin project's icon-picker component, requests to the Iconify API should be deduplicated using a PENDING_REQUESTS cache to prevent multiple simultaneous requests for the same prefix, and should include a 10-second timeout using AbortController.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5075
File: packages/effects/common-ui/src/components/api-select/api-select.vue:61-62
Timestamp: 2024-12-09T04:41:58.914Z
Learning: 在文件 `packages/effects/common-ui/src/components/api-select/api-select.vue` 的组件 `ApiSelect` 中,`childrenField` 的默认值设置为空字符串是有意的,因为并非所有组件都使用树形数据。
apps/web-naive/src/adapter/component/index.ts (6)
Learnt from: ArthurDarkstone
PR: vbenjs/vue-vben-admin#4807
File: docs/src/components/common-ui/vben-vxe-table.md:65-84
Timestamp: 2024-11-05T10:34:13.846Z
Learning: In `docs/src/components/common-ui/vben-vxe-table.md`, the code examples are provided within markdown scripts for documentation and do not require import statements or explicit type annotations.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5397
File: playground/src/bootstrap.ts:23-30
Timestamp: 2025-01-15T04:29:13.944Z
Learning: In the Vue-Vben-Admin project, commented code in bootstrap.ts showing modal and drawer configuration examples should be preserved as reference documentation, even after moving to CSS variables for z-index management.
Learnt from: ArthurDarkstone
PR: vbenjs/vue-vben-admin#4807
File: docs/src/_env/adapter/vxe-table.ts:0-0
Timestamp: 2024-11-05T10:38:55.166Z
Learning: In the documentation demos, such as in `docs/src/_env/adapter/vxe-table.ts`, it's acceptable to have less strict type safety and minimal error handling in custom renderers.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5587
File: playground/src/views/examples/loading/index.vue:15-18
Timestamp: 2025-02-23T04:21:24.691Z
Learning: Chinese text in the description of the loading component example (`playground/src/views/examples/loading/index.vue`) is intentionally kept without i18n support.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5446
File: packages/effects/common-ui/src/components/icon-picker/icons.ts:0-0
Timestamp: 2025-01-21T05:03:44.466Z
Learning: In the vue-vben-admin project's icon-picker component, requests to the Iconify API should be deduplicated using a PENDING_REQUESTS cache to prevent multiple simultaneous requests for the same prefix, and should include a 10-second timeout using AbortController.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5075
File: packages/effects/common-ui/src/components/api-select/api-select.vue:61-62
Timestamp: 2024-12-09T04:41:58.914Z
Learning: 在文件 `packages/effects/common-ui/src/components/api-select/api-select.vue` 的组件 `ApiSelect` 中,`childrenField` 的默认值设置为空字符串是有意的,因为并非所有组件都使用树形数据。
playground/src/adapter/component/index.ts (4)
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5587
File: playground/src/views/examples/loading/index.vue:15-18
Timestamp: 2025-02-23T04:21:24.691Z
Learning: Chinese text in the description of the loading component example (`playground/src/views/examples/loading/index.vue`) is intentionally kept without i18n support.
Learnt from: ArthurDarkstone
PR: vbenjs/vue-vben-admin#4807
File: docs/src/components/common-ui/vben-vxe-table.md:65-84
Timestamp: 2024-11-05T10:34:13.846Z
Learning: In `docs/src/components/common-ui/vben-vxe-table.md`, the code examples are provided within markdown scripts for documentation and do not require import statements or explicit type annotations.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5446
File: packages/effects/common-ui/src/components/icon-picker/icons.ts:0-0
Timestamp: 2025-01-21T05:03:44.466Z
Learning: In the vue-vben-admin project's icon-picker component, requests to the Iconify API should be deduplicated using a PENDING_REQUESTS cache to prevent multiple simultaneous requests for the same prefix, and should include a 10-second timeout using AbortController.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5075
File: packages/effects/common-ui/src/components/api-select/api-select.vue:61-62
Timestamp: 2024-12-09T04:41:58.914Z
Learning: 在文件 `packages/effects/common-ui/src/components/api-select/api-select.vue` 的组件 `ApiSelect` 中,`childrenField` 的默认值设置为空字符串是有意的,因为并非所有组件都使用树形数据。
apps/web-antd/src/adapter/component/index.ts (3)
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5075
File: packages/effects/common-ui/src/components/api-select/api-select.vue:61-62
Timestamp: 2024-12-09T04:41:58.914Z
Learning: 在文件 `packages/effects/common-ui/src/components/api-select/api-select.vue` 的组件 `ApiSelect` 中,`childrenField` 的默认值设置为空字符串是有意的,因为并非所有组件都使用树形数据。
Learnt from: ArthurDarkstone
PR: vbenjs/vue-vben-admin#4807
File: docs/src/components/common-ui/vben-vxe-table.md:65-84
Timestamp: 2024-11-05T10:34:13.846Z
Learning: In `docs/src/components/common-ui/vben-vxe-table.md`, the code examples are provided within markdown scripts for documentation and do not require import statements or explicit type annotations.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5446
File: packages/effects/common-ui/src/components/icon-picker/icons.ts:0-0
Timestamp: 2025-01-21T05:03:44.466Z
Learning: In the vue-vben-admin project's icon-picker component, requests to the Iconify API should be deduplicated using a PENDING_REQUESTS cache to prevent multiple simultaneous requests for the same prefix, and should include a 10-second timeout using AbortController.
apps/web-naive/src/views/demos/form/modal.vue (3)
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5397
File: playground/src/bootstrap.ts:23-30
Timestamp: 2025-01-15T04:29:13.944Z
Learning: In the Vue-Vben-Admin project, commented code in bootstrap.ts showing modal and drawer configuration examples should be preserved as reference documentation, even after moving to CSS variables for z-index management.
Learnt from: ArthurDarkstone
PR: vbenjs/vue-vben-admin#4807
File: docs/src/components/common-ui/vben-vxe-table.md:65-84
Timestamp: 2024-11-05T10:34:13.846Z
Learning: In `docs/src/components/common-ui/vben-vxe-table.md`, the code examples are provided within markdown scripts for documentation and do not require import statements or explicit type annotations.
Learnt from: mynetfan
PR: vbenjs/vue-vben-admin#5013
File: docs/src/components/layout-ui/page.md:31-31
Timestamp: 2024-12-04T04:41:30.161Z
Learning: 在 Vben Admin 项目的 `Page` 组件中,`extra` 内容仅通过 slots 提供,没有对应的 prop 属性。
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
🔇 Additional comments (16)
apps/web-ele/src/adapter/component/index.ts (2)
11-11
: Clean import optimization.Removing
getCurrentInstance
is appropriate since the new Proxy-based approach eliminates the need for lifecycle-dependent method copying.
136-144
: Excellent Proxy-based method forwarding implementation.The new approach elegantly solves the Vue warning issue by using a Proxy to dynamically forward method calls to the inner component. The implementation correctly handles:
- Safe property access with optional chaining (
innerRef.value?.[key]
)- Property existence checks with null safety (
key in (innerRef.value || {})
)- Dynamic forwarding without manual copying or lifecycle dependencies
This is a significant improvement over the previous
$nextTick
+ manual copying approach.apps/web-antd/src/adapter/component/index.ts (2)
11-11
: Consistent import cleanup across adapters.The removal of
getCurrentInstance
aligns with the new Proxy-based approach and maintains consistency with other adapter files.
79-87
: Consistent Proxy implementation across adapters.The method forwarding implementation is identical to other adapter files, ensuring consistent behavior across different UI library adapters. The Proxy correctly handles dynamic method access without Vue warnings.
playground/src/adapter/component/index.ts (2)
11-11
: Import optimization consistent with other adapters.The removal of
getCurrentInstance
follows the same pattern as other adapter files in this refactoring.
79-96
: Excellent documentation of the refactoring approach.This file provides valuable insight by preserving the old implementation as comments, clearly showing the evolution from manual method copying to Proxy-based forwarding. The commented code demonstrates why the change was necessary:
- Complex lifecycle management with
getCurrentInstance()
and$nextTick
- Manual iteration and copying of function properties
- Replaced by a clean, reactive Proxy solution
This serves as excellent documentation for future maintainers.
apps/web-naive/src/views/demos/form/basic.vue (4)
2-2
: Proper modal integration setup.Adding
useVbenModal
import is the correct approach for integrating modal functionality into the existing form demo.
9-9
: Clean component organization.Importing the modal as a separate component (
modalDemo
) maintains good separation of concerns and reusability.
149-151
: Correct modal hook usage.The
useVbenModal
implementation properly connects to the imported modal component, following the established pattern for modal integration.
161-167
: Well-integrated modal trigger and component.The modal button is appropriately placed in the card header with proper styling (
ml-2
for spacing), and the<Modal />
component is correctly positioned in the template. ThemodalApi.open()
call follows the expected API pattern.apps/web-naive/src/adapter/component/index.ts (2)
11-11
: Final adapter import cleanup completed.The removal of
getCurrentInstance
completes the consistent refactoring across all adapter component files.
82-90
: Consistent Proxy implementation across all adapters.This completes the unified refactoring of method forwarding across all UI library adapters (Element Plus, Ant Design Vue, Naive UI, and Playground). The implementation maintains identical behavior and safety patterns, ensuring consistent component method exposure without Vue warnings.
apps/web-naive/src/views/demos/form/modal.vue (4)
1-8
: Component setup looks good.The component follows Vue 3 composition API best practices with proper TypeScript setup and clear component naming.
52-55
: Clarify the commented modal close behavior.The commented
modalApi.close()
line might cause confusion about the intended behavior. If this is intentional for demo purposes, consider adding a comment explaining why.Is the modal supposed to stay open after form submission for demo purposes, or should it close? If it should close, uncomment line 54. If it should stay open intentionally, add a comment explaining the behavior:
onConfirm: async () => { await formApi.validateAndSubmitForm(); - // modalApi.close(); + // Keep modal open for demo purposes - remove this comment and uncomment the line below for production use + // modalApi.close(); },
56-65
: Modal data handling logic is correct.The open state change handler properly retrieves data and populates form fields when the modal opens.
67-71
: Template structure is clean and appropriate.The template correctly renders the modal containing the form component with proper component composition.
使用Proxy解决组件封装时透传组件暴露的方法导致的vue警告,并添加相关示例 Closes vbenjs#6334
abf651e
to
8da15a0
Compare
使用Proxy解决组件封装时透传组件暴露的方法导致的vue警告,并添加相关示例
Closes #6334
Description
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
New Features
Refactor