fix: form submission not appropriate #5126
Conversation
|
|
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
packages/@core/ui-kit/form-ui/src/vben-use-form.vueOops! Something went wrong! :( ESLint: 9.16.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs WalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
![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
🧹 Outside diff range and nitpick comments (2)
packages/@core/ui-kit/form-ui/src/vben-use-form.vue (2)
69-73: Great fix for the premature form submission issue!The implementation correctly addresses the issue by:
- Ensuring the watch is set up only after component mount
- Using
nextTickto wait for form initialization- Maintaining deep watching for nested form values
Consider enhancing the comment to document the specific issue this fixes:
- // 只在挂载后开始监听，form.values会有一个初始化的过程 + // Only start watching after mount to prevent premature form submission during initialization
69-73: Consider adding error handling for the async operationWhile the implementation is solid, it would be more robust with proper error handling:
onMounted(async () => { // Only start watching after mount to prevent premature form submission during initialization - await nextTick(); - watch(() => form.values, handleValuesChangeDebounced, { deep: true }); + try { + await nextTick(); + watch(() => form.values, handleValuesChangeDebounced, { deep: true }); + } catch (error) { + console.error('Failed to setup form value watch:', error); + } });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/@core/ui-kit/form-ui/src/vben-use-form.vue(2 hunks)
🔇 Additional comments (2)
packages/@core/ui-kit/form-ui/src/vben-use-form.vue (2)
9-11: LGTM! Import changes align with the implementation needs
The addition of nextTick, onMounted, and cloneDeep imports supports the new implementation for proper form value handling and lifecycle management.
64-67: Consider validating the debounce timeout value
The implementation looks good, using cloneDeep to ensure immutable form values and debouncing to optimize performance. However, please verify if the 300ms debounce timeout provides the optimal balance between responsiveness and performance for your use case.
Let's check if there are any established debounce timeout patterns in the codebase:
✅ Verification successful
The 300ms debounce timeout is consistent with codebase patterns
The debounce timeout of 300ms in the form value change handler aligns well with the established patterns in the codebase, where similar debounce timeouts range from 150ms to 200ms for various UI interactions. The slightly higher timeout (300ms) is appropriate for form value changes as it provides a good balance between responsiveness and performance, especially considering the additional processing with cloneDeep and potential form submissions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other debounce timeout values for consistency
rg -A 1 "useDebounceFn.*\(" --type ts --type vue
Length of output: 78
Script:
#!/bin/bash
# Search for other debounce timeout values for consistency, without file type restrictions
rg -A 1 "useDebounceFn.*\("
Length of output: 1253
Script:
#!/bin/bash
# Search for debounce timeout values in the found files
rg -A 2 "useDebounceFn.*\(" packages/effects/plugins/src/echarts/use-echarts.ts packages/@core/preferences/src/preferences.ts packages/@core/ui-kit/tabs-ui/src/use-tabs-view-scroll.ts packages/@core/composables/src/use-layout-style.ts
Length of output: 1704
Description
修复表单在开启submitOnChange后在挂载后立即就会不适宜地提交一次的问题.
fix #5124
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
Bug Fixes