-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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: form 表单不支持field.xxx.xx格式的defaultValue配置 #4965
Conversation
WalkthroughThe pull request introduces several modifications to enhance the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 1
🧹 Outside diff range and nitpick comments (3)
packages/@core/base/shared/src/utils/index.ts (1)
18-18
: Consider adding JSDoc documentation for the new export.To maintain consistency and improve maintainability, consider adding documentation for the new export.
+/** Safely sets a value at path of object. If a portion of path doesn't exist, it's created. */ export { default as set } from 'lodash.set';
packages/@core/ui-kit/form-ui/src/use-form-context.ts (1)
44-46
: Consider documenting nested field supportWhile the implementation is solid, consider:
- Adding JSDoc comments explaining the support for nested paths
- Updating form documentation to highlight this feature
- Reviewing other form features (like validation rules, transformers) to ensure consistent support for nested paths
playground/src/views/examples/form/basic.vue (1)
294-299
: Consider enhancing the layout and example.The new field successfully demonstrates the fix for supporting
field.xxx.xx
format. However, consider these improvements:
- Add
formItemClass
to maintain consistent layout with other fields- Consider a more comprehensive example with actual nested object values
{ component: 'Input', defaultValue: 'field4.path', fieldName: 'field4.path', label: 'field4.path', + formItemClass: 'col-start-1', },
Additionally, consider adding another example with deeper nesting and actual object values:
+ { + component: 'Input', + defaultValue: { nested: { value: 'test' } }, + fieldName: 'field5.nested.value', + label: 'Nested Object Example', + formItemClass: 'col-start-2', + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
packages/@core/base/shared/package.json
(1 hunks)packages/@core/base/shared/src/utils/index.ts
(1 hunks)packages/@core/ui-kit/form-ui/src/use-form-context.ts
(2 hunks)playground/src/views/examples/form/basic.vue
(1 hunks)pnpm-workspace.yaml
(2 hunks)
🔇 Additional comments (5)
packages/@core/base/shared/src/utils/index.ts (1)
18-18
: LGTM! Verify type definitions are properly configured.
The addition of lodash.set
is appropriate for handling nested property paths in form configurations. This aligns well with the PR objective to support field.xxx.xx
format.
Let's verify the type definitions are properly configured:
✅ Verification successful
Type definitions are properly configured - good to go!
The verification shows that:
lodash.set
is correctly added as a dependency in the shared package@types/lodash.set
is properly configured in devDependencies- The package is only imported in the utils/index.ts file as intended
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if lodash.set types are properly configured in package.json
# Expected: Should find @types/lodash.set in devDependencies
# Search for package.json files that might contain lodash.set configuration
fd package.json | while read -r file; do
echo "Checking $file:"
cat "$file" | jq -r '.dependencies["lodash.set"] // empty, .devDependencies["@types/lodash.set"] // empty'
done
Length of output: 10881
Script:
#!/bin/bash
# Let's check the root package.json and the shared package's package.json more thoroughly
echo "=== Root package.json ==="
cat package.json | jq '.dependencies,.devDependencies'
echo -e "\n=== Shared package's package.json ==="
cat packages/@core/base/shared/package.json | jq '.dependencies,.devDependencies'
# Also check if lodash.set is already being used somewhere
echo -e "\n=== Files using lodash.set ==="
rg "lodash\.set" --type ts
Length of output: 1962
packages/@core/ui-kit/form-ui/src/use-form-context.ts (2)
6-6
: LGTM: Import of set
utility is appropriate
The addition of set
from shared utils (backed by lodash.set) is a good choice for handling nested object properties in form fields.
44-46
: Implementation correctly supports nested field paths
The use of set
utility properly handles both simple and nested property paths in form field names, directly addressing the PR objective.
Let's verify the behavior with complex nested paths:
✅ Verification successful
Implementation correctly supports nested field paths
The verification confirms that nested field paths are properly supported:
- Found example usage in playground with
field4.path
demonstrating nested path support - The
set
utility is correctly used inuse-form-context.ts
for bothinitialValues
andzodObject
- The form API implementation (
form-api.ts
) includes proper handling of nested fields through:- Value setting with deep merge support
- Field validation
- Form submission with nested structures
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing usage patterns of nested form fields to ensure compatibility
# Look for form schema definitions with nested paths
rg -A 5 "fieldName: ['\"].*\..*['\"]"
# Look for form default value assignments with nested paths
rg -A 5 "defaultValue.*{.*\..*}"
Length of output: 581
Script:
#!/bin/bash
# Let's try a different approach to find nested field patterns and their usage
# Look for field name patterns with dots using ast-grep
ast-grep --pattern 'fieldName: $FIELD' --pattern-type js
# Search for set utility usage with nested paths
rg -A 3 "set\(.+,.+\..+,"
# Look for form-related test files that might contain nested field scenarios
fd "form.*test|test.*form" --type f
# Search for form schema definitions in example files
fd "form" --type f --exec rg -l "schema.*=.*\[" {} \;
Length of output: 987
Script:
#!/bin/bash
# Let's examine the test file and form API implementation for nested field handling
# Check form API tests for nested field scenarios
cat packages/@core/ui-kit/form-ui/__tests__/form-api.test.ts
# Check form API implementation
cat packages/@core/ui-kit/form-ui/src/form-api.ts
# Look for actual usage of nested fields in the playground
cat playground/src/views/examples/form/basic.vue
Length of output: 21223
packages/@core/base/shared/package.json (2)
89-89
: LGTM! Good choice using lodash.set for nested property handling.
The addition of lodash.set
and its type definitions is a suitable solution for handling nested object paths in form configurations. This aligns well with the PR's objective to support field.xxx.xx
format in form defaultValue.
Also applies to: 96-96
Line range hint 27-31
: Verify the utils export configuration.
Since lodash.set is being added and exported through utils, let's verify that the export configuration in utils/index.ts matches the package.json configuration.
✅ Verification successful
Export configuration for utils is correctly set up
The verification confirms that:
- The
lodash.set
is properly exported inutils/index.ts
viaexport { default as set } from 'lodash.set'
- The package.json configuration for utils exports correctly maps the source and distribution paths:
- Types: "./src/utils/index.ts"
- Development: "./src/utils/index.ts"
- Default: "./dist/utils/index.mjs"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the lodash.set export in utils/index.ts
# Check if the set function is properly exported
rg -l "export.*set.*from.*lodash\.set" "src/utils/"
# Verify there are no duplicate exports of 'set'
rg -c "export.*set" "src/utils/"
Length of output: 186
Script:
#!/bin/bash
# First, let's find the correct path to utils directory
fd -t d "utils" packages/@core/base/shared
# Then check the content of index.ts in utils directory
fd -t f "index.ts" packages/@core/base/shared/src/utils -x cat {}
# Also verify the exports configuration
cat packages/@core/base/shared/package.json
Length of output: 3417
ci没通过 |
哦,我看到那里没通过了,我处理一下再重新提交 |
|
这个功能 在 5.5.0 好像并没有发布上去 use-form-context.ts 没看到修改后的代码 |
Description
Form 表单useVbenForm 无法设置表单字段 field.xx.xx 的defaultValue的默认值配置
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
Release Notes
New Features
set
function from lodash to enhance utility functions for manipulating object properties.Bug Fixes
Chores