-
Notifications
You must be signed in to change notification settings - Fork 8.4k
feat: improved formApi for component instance support #5689
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
Conversation
* 改进表单API以支持组件实例的获取,以及焦点字段的获取
|
WalkthroughThis PR introduces multiple refactors and feature enhancements across several modules. The primary changes refactor the Changes
Sequence Diagram(s)sequenceDiagram
participant Parent
participant Wrapper as withDefaultPlaceholder
participant Setup as Setup Function
participant Vue as Vue Lifecycle ($nextTick)
participant API as Public API
Parent->>Wrapper: Call withDefaultPlaceholder(component, type)
Wrapper->>Setup: Initialize component via defineComponent
Setup->>Setup: Create innerRef for component instance
Setup->>Vue: Retrieve current instance & register $nextTick callback
Vue-->>Setup: Execute callback post-mount
Setup->>API: Populate public API with innerRef methods
Setup-->>Parent: Return enhanced component with public API
sequenceDiagram
participant Field as Form Field Component
participant Context as ComponentRefMap (via inject/provide)
participant FormAPI as FormApi
Field->>Context: Register field reference (via watch)
Field->>Context: On unmount, remove field reference (onUnmounted)
FormAPI->>Context: Lookup field reference via getFieldComponentRef
Context-->>FormAPI: Return component instance (if focused)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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 (
|
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 (5)
playground/src/views/_core/authentication/login.vue (1)
111-121: Verify the async usage and consider a more consistent error-handling approach.Currently, the function is marked
asyncbut uses a.catch()on the promise instead oftry/catch. You may either removeasyncor use an explicitawait/try-catchto handle errors more uniformly.apps/web-antd/src/adapter/component/index.ts (1)
45-68: Refactored component wrapper looks good but watch for timing issues.The pattern of capturing child methods on
nextTickis effective for exposing the underlying API, though there's a brief window before those methods are assigned topublicApi. If immediate parent usage is necessary, consider verifying that parent calls happen after thenextTicklifecycle. Otherwise, this approach is a clean way to forward methods from the wrapped component.playground/src/adapter/component/index.ts (1)
45-68: Consider adding type safety and ref null checks.
While the pattern of exposing the child component’s methods is valid, relying onanyprops and iterating overinnerRef.valuewithout null checks can lead to runtime errors or type mismatches in certain edge cases.Below is an optional improvement:
- Replace
props: anywith a proper props type or generics to ensure type safety.- Safeguard usage of
innerRef.valuewith optional chaining or an existence check:- for (const key in innerRef.value) { - if (typeof innerRef.value[key] === 'function') { - publicApi[key] = innerRef.value[key]; - } - } + const inner = innerRef.value; + if (inner) { + for (const key in inner) { + if (typeof inner[key] === 'function') { + publicApi[key] = inner[key]; + } + } + }Consider architectural unification.
You have repeated versions ofwithDefaultPlaceholderacross multiple files (playground,web-ele,web-naive). Factor them out into a shared utility to promote DRY and consistent maintenance if possible.apps/web-ele/src/adapter/component/index.ts (1)
41-64: Enhance type definitions and avoid potential null references.
Currently,propsis typed asany, andinnerRef.valuemight be undefined at render time. For added resilience and clarity:
- Define a more explicit props interface if feasible.
- Use defensively optional chaining when iterating methods from
innerRef.valueto prevent crashes.- for (const key in innerRef.value) { - if (typeof innerRef.value[key] === 'function') { - publicApi[key] = innerRef.value[key]; - } - } + const inner = innerRef.value; + if (inner) { + for (const key in inner) { + if (typeof inner[key] === 'function') { + publicApi[key] = inner[key]; + } + } + }Reduce duplication across adapters.
Since an almost identicalwithDefaultPlaceholderis used across multiple adapters (web-antd,web-naive, etc.), consider centralizing this logic in a shared module.apps/web-naive/src/adapter/component/index.ts (1)
41-64: Improve null safety and unify the pattern.
To ensure stability and maintainability:
- Add a quick null check for
innerRef.valueto avoid unexpected runtime errors.- Encourage a typed props structure rather than
props: any.- Factor out the
withDefaultPlaceholderlogic into a shared file to reduce duplication across multiple adapters.- for (const key in innerRef.value) { - if (typeof innerRef.value[key] === 'function') { - publicApi[key] = innerRef.value[key]; - } - } + const inner = innerRef.value; + if (inner) { + for (const key in inner) { + if (typeof inner[key] === 'function') { + publicApi[key] = inner[key]; + } + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
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)docs/src/components/common-ui/vben-form.md(1 hunks)package.json(1 hunks)packages/@core/ui-kit/form-ui/src/form-api.ts(4 hunks)packages/@core/ui-kit/form-ui/src/form-render/form-field.vue(3 hunks)packages/@core/ui-kit/form-ui/src/use-form-context.ts(1 hunks)packages/@core/ui-kit/form-ui/src/vben-use-form.vue(2 hunks)playground/src/adapter/component/index.ts(2 hunks)playground/src/views/_core/authentication/login.vue(2 hunks)playground/src/views/examples/form/api.vue(5 hunks)pnpm-workspace.yaml(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (28)
package.json (1)
102-102: Package manager version updated.The package manager version has been updated from
9.15.6to9.15.7. This is likely a routine maintenance update that includes bug fixes or minor enhancements.packages/@core/ui-kit/form-ui/src/vben-use-form.vue (4)
20-24: Import updated to include component reference map provider.The import statement has been updated to include
provideComponentRefMap, which is used to provide the component reference map to child components.
36-37: Component reference map initialization.A new Map is initialized to store component references by field name. This is a key part of the new feature allowing component instances to be retrieved.
41-41: Providing component reference map to descendant components.The component reference map is provided to the component tree using Vue's dependency injection system, making it accessible to field components.
43-43: Passing component reference map to FormApi.The component reference map is now passed to the FormApi's mount method, enabling the API to access component instances by field name.
packages/@core/ui-kit/form-ui/src/use-form-context.ts (1)
23-24: New context for component reference management.Added context functions for injecting and providing a map of component references, which enables component instance retrieval throughout the form component tree.
packages/@core/ui-kit/form-ui/src/form-render/form-field.vue (3)
6-6: Added onUnmounted lifecycle hook import.The onUnmounted hook is now imported to support proper cleanup of component references when fields are removed.
21-21: Added import for component reference map injection.Importing the injectComponentRefMap function to access the component reference map provided by the parent form.
271-279: Component reference tracking implementation.This code adds crucial functionality to track field component references:
- It injects the component reference map from the form context
- It watches for changes to the field component reference and updates the map
- It cleans up references when components are unmounted to prevent memory leaks
This implementation is essential for the new feature that allows retrieving component instances by field name.
docs/src/components/common-ui/vben-form.md (1)
282-299: Good documentation coverage for newly introduced methods.The added rows effectively communicate the usage and availability of
getFieldComponentRefandgetFocusedField. This section clearly aligns with the updated form API functionalities and helps developers understand how to interact with component instances and focus management within forms.playground/src/views/_core/authentication/login.vue (2)
3-5: No issues found with new imports.Importing
RecordableanduseTemplateRefis clear and helps maintain type safety while exposing component references.
108-110: Appropriate usage of template ref for authentication component.Storing a reference to the
AuthenticationLogincomponent instance allows for easier calling of methods likegetFormApi.playground/src/views/examples/form/api.vue (1)
2-3: Neat addition of component reference handling and focus control.• The extended action union for
'componentRef'fits seamlessly into the existing structure.
• Retrieving the dropdown’s component instance and callingfocus()showcasesgetFieldComponentRefeffectively.
• The new button for testing this interaction is a practical demo entry point.Also applies to: 83-87, 135-139, 268-268
playground/src/adapter/component/index.ts (3)
6-6: Import ofComponentis correct.
The import from'vue'aligns well with the upcoming usage ofdefineComponent.
9-9: Import ofRecordableis correct.
This type import is valid for storing the methods in a dictionary-like object.
11-11: Imports for Vue composition API are correct.
UsingdefineComponent,getCurrentInstance,h, andrefis appropriate for this refactor.apps/web-ele/src/adapter/component/index.ts (2)
6-6: Import ofComponentis correct.
No issues found with the import from'vue'.
11-11: Imports for Vue composition API are valid.
The usage matches the newdefineComponentapproach.apps/web-naive/src/adapter/component/index.ts (3)
6-6: Import ofComponentis correct.
Consistent with the refactoreddefineComponentusage.
9-9: Import ofRecordablealigns with usage.
Using a record type helps store arbitrary method mappings.
11-11: Vue composition API imports are valid.
The updated approach correctly leveragesdefineComponent,getCurrentInstance, andref.packages/@core/ui-kit/form-ui/src/form-api.ts (6)
8-8: Good addition of Vue's ComponentPublicInstance importThis import is necessary for the new functionality that allows accessing component instances by field name.
61-64: Well-structured component reference map implementationThe private property
componentRefMapis appropriately initialized as an empty Map and will store component instances mapped by their field names. The comment clearly explains its purpose in Chinese.
95-106: Good implementation of component reference retrieval methodThe
getFieldComponentRefmethod is well-typed with generic parameter support, allowing for flexible component instance retrieval. The implementation correctly handles the case when a field component isn't found in the map.
108-133: Thorough implementation of the focused field detectionThe
getFocusedFieldmethod effectively handles different component types:
- Direct HTMLElement instances
- Vue component instances with $el property
- Checks both direct focus and child element focus
This implementation aligns well with the PR objective of facilitating custom validation functions that need to know when a field loses focus.
193-193: Updated mount method signature to support component reference mapThe mount method signature has been appropriately updated to accept the component reference map as a parameter.
200-200: Proper assignment of the component reference map during mountingThe form API correctly stores the provided component reference map, which enables the new component instance retrieval functionality.
pnpm-workspace.yaml (1)
21-22: Version updates for dependenciesThese changes update various package dependencies to newer versions. While not directly related to the form API enhancement functionality, keeping dependencies updated is a good practice for security and compatibility reasons.
Also applies to: 24-27, 29-30, 34-35, 39-39, 48-48, 53-54, 62-64, 68-68, 81-82, 88-90, 96-98, 104-104, 123-123, 129-129, 136-136, 143-143, 147-147, 166-168, 178-178, 183-183
Description
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
New Features
Documentation
Chores