fix: form component events bind #5137
Conversation
mynetfan
requested review from
anncwb,
vince292007 and
a team
as code owners
|
|
Warning Rate limit exceeded@mynetfan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 14 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe pull request modifies the form configuration across multiple files, focusing on event listener handling for form components. The changes primarily involve removing or adjusting the Changes
Assessment against linked issues
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web-ele/src/adapter/form.ts(0 hunks)apps/web-naive/src/adapter/form.ts(0 hunks)packages/@core/ui-kit/form-ui/src/config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/web-naive/src/adapter/form.ts
- apps/web-ele/src/adapter/form.ts
🔇 Additional comments (1)
packages/@core/ui-kit/form-ui/src/config.ts (1)
48-49: Consider a more targeted solution for the Select component issue.
While changing the default listener behavior fixes the Select component's filtering issue, this is a broad change that affects all form components. Consider these alternatives:
- Create component-specific listener configurations
- Add a separate configuration for filtering behavior in Select components
Let's verify the impact on other components:
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/@core/ui-kit/form-ui/src/types.ts (1)
152-161: Document the rationale for disabled listenersThe JSDoc comments for
disabledOnChangeListeneranddisabledOnInputListenershould explain why these listeners are disabled by default, particularly in relation to the Select component's filtering behavior. This will help maintainers understand the reasoning behind these defaults./** * 是否禁用所有表单项的change事件监听 + * Disabled by default to prevent unintended form updates during component-specific operations + * (e.g., Select component's filtering) * @default true */ disabledOnChangeListener?: boolean; /** * 是否禁用所有表单项的input事件监听 + * Disabled by default to prevent unintended form updates during component-specific operations + * (e.g., Select component's filtering) * @default true */ disabledOnInputListener?: boolean;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/@core/ui-kit/form-ui/src/types.ts(1 hunks)
🔇 Additional comments (1)
packages/@core/ui-kit/form-ui/src/types.ts (1)
154-154: Verify the impact of changing default listener behavior
Changing the default values of disabledOnChangeListener and disabledOnInputListener to true could affect existing form implementations. While this change fixes the Select component's filtering issue, we should verify that it doesn't introduce regressions in other components.
Also applies to: 159-159
✅ Verification successful
Let me analyze the results and generate additional verification scripts to understand the impact better.
The previous results show that the default values are consistently set to true in the codebase, but we need to verify test coverage and component integration to ensure this change aligns with expected behavior.
Let me check the form tests and documentation specifically related to these listener properties to understand the intended behavior.
The previous results show consistent usage of these properties, but we need to verify the test coverage and documentation to ensure this change is intentional and properly tested.
Based on the comprehensive review of the test files, documentation, and codebase, I can now generate the final response:
Default listener behavior change is safe and intentional
The change to set disabledOnChangeListener and disabledOnInputListener to true by default is safe because:
- The form API tests show that form behavior is well-tested and the change doesn't affect core form functionality
- The documentation indicates that form event handling is configurable through
commonConfig - Only one explicit usage was found in
docs/src/_env/adapter/form.tswhich already sets these properties totrue - This change aligns with the PR's goal of fixing Select component's input keyword filtering issues
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for form configurations that might be affected by the new defaults
# Look for form configurations that explicitly set these properties
echo "Searching for explicit listener configurations..."
rg -A 2 "disabledOnChangeListener|disabledOnInputListener" --type ts
# Look for form usages that might be affected
echo "Searching for form component usages..."
ast-grep --pattern 'const $_ = useForm($_)'
Length of output: 2149
Script:
#!/bin/bash
# Search for form usage patterns in test files to understand expected behavior
echo "Searching for form tests..."
rg -l "test|spec|describe.*form" --type ts
# Search for Select component implementations to understand the relationship
echo "Searching for Select component usage with forms..."
rg -A 5 "Select.*form" --type ts
# Look for any migration guides or documentation about form behavior
echo "Checking documentation..."
fd -e md -e mdx | xargs rg -l "form|listener|event"
Length of output: 4234
Script:
#!/bin/bash
# Check form API tests for listener behavior expectations
echo "Checking form API tests..."
cat "packages/@core/ui-kit/form-ui/__tests__/form-api.test.ts"
# Check form documentation
echo "Checking form documentation..."
cat "docs/src/components/common-ui/vben-form.md"
# Look for any recent changes to form behavior
echo "Checking recent form-related changes..."
rg "disabledOnChangeListener|disabledOnInputListener" "packages/@core/ui-kit/form-ui/src/form-api.ts"
Length of output: 16944
mynetfan
changed the title
Description
修复各组件库的Select在启用过滤功能时，输入的关键词会被错误地更新到表单值的问题。
fix: #5136
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