fix(breadcrumb): [breadcrumb] update size prop type and add validator #3037
Conversation
WalkthroughThis pull request restricts the allowed values for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
examples/sites/demos/apis/breadcrumb.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/vue/src/breadcrumb-item/src/index.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/vue/src/breadcrumb/src/index.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
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 (
|
gweesin
changed the title
WalkthroughThis pull request addresses a bugfix for the breadcrumb component by updating the Changes
|
![petercat-assistant[bot]](https://avatars.githubusercontent.com/in/875736?s=60&v=4){kind=small}
| { | ||
| name: 'size', | ||
| type: 'string', | ||
| type: "'medium'", |
There was a problem hiding this comment.
The change from type: 'string' to type: 'medium' seems incorrect. The type should likely remain string to allow for different size values, unless 'medium' is intended as a default or constant value.
![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
🧹 Nitpick comments (2)
packages/vue/src/breadcrumb/src/index.ts (1)
38-42: Validator implementation ensures consistent size property values.The updated validator restricts the
sizeproperty to only accept 'medium' or an empty string, which aligns with the API documentation changes. The commented-out values ('large', 'small', 'mini') indicate these were previously valid options but are now disabled.To improve clarity for future maintainers, consider adding a comment explaining why only 'medium' and empty string are valid now. Also, for better readability, you could extract the allowed sizes into a constant:
+// Only 'medium' size is currently supported as per issue #3004 +const ALLOWED_SIZES = ['medium', '']; size: { type: String, default: '', - validator: (val: string) => [/* 'large', */ 'medium', /* 'small', 'mini', */ ''].includes(val) + validator: (val: string) => ALLOWED_SIZES.includes(val) },examples/sites/demos/apis/breadcrumb.js (1)
43-45: Type restriction for 'size' prop is consistent with validator changes.The type has been correctly updated from a generic string to specifically "'medium'" to restrict the allowed values.
To complement this change, consider updating the description to explicitly mention that only 'medium' size is supported:
desc: { - 'zh-CN': '设置面包屑尺寸大小', - 'en-US': 'Set breadcrumb size' + 'zh-CN': '设置面包屑尺寸大小，当前仅支持 medium', + 'en-US': 'Set breadcrumb size, currently only supports medium' },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
examples/sites/demos/apis/breadcrumb.js(2 hunks)packages/vue/src/breadcrumb-item/src/index.ts(1 hunks)packages/vue/src/breadcrumb/src/index.ts(1 hunks)
🔇 Additional comments (2)
examples/sites/demos/apis/breadcrumb.js (1)
109-111: Consistent type restriction for BreadcrumbItem's 'size' prop.The type change from a generic string to "'medium'" mirrors the same change made for the Breadcrumb component, ensuring consistency across related components.
packages/vue/src/breadcrumb-item/src/index.ts (1)
22-26:✅ Verification successful
Validator implementation consistently restricts size values across components.
The validator implementation matches the one in the Breadcrumb component, ensuring that both components handle the
sizeproperty in the same way.Let's check if there are any usages of sizes other than 'medium' or empty string in the codebase that might need updating:
🏁 Script executed:
#!/bin/bash # Find any usage of 'large', 'small', or 'mini' sizes for breadcrumb components that might need to be updated # Search for breadcrumb size usages in the codebase echo "Searching for breadcrumb size usages in Vue templates or JavaScript/TypeScript files..." rg -i "size\s*=\s*['\"](?:large|small|mini)['\"]" --type vue --type js --type ts rg -i ":size\s*=\s*['\"](?:large|small|mini)['\"]" --type vue rg -i "size:\s*['\"](?:large|small|mini)['\"]" --type js --type ts echo "Searching for breadcrumb examples or demos with size props..." rg -i "breadcrumb.*size.*(?:large|small|mini)" --type vue --type js --type tsLength of output: 3136
Below is a refined shell script that avoids the “unrecognized file type: vue” error by using explicit globs. This script targets breadcrumb-related directories to verify that no instances using the disallowed size values (‘large’, ‘small’, or ‘mini’) remain—and thus that the validator in
packages/vue/src/breadcrumb-item/src/index.tsis indeed consistent with the Breadcrumb component.Please run the following script and share its output so we can confirm that only the allowed values (i.e.
'medium'or'') are used in the breadcrumb components:Once we’ve confirmed that no breadcrumb files use disallowed size values, we can confidently state that the validator implementation is consistent across components.
Action: Please run the above script and verify the output.
🏁 Script executed:
#!/bin/bash echo "Searching for invalid size values in Breadcrumb components (Vue and TS files)..." echo ">> Checking in packages/vue/src/breadcrumb:" rg -i --glob "*.vue" "size\s*=\s*['\"](?:large|small|mini)['\"]" packages/vue/src/breadcrumb rg -i --glob "*.vue" "size:\s*['\"](?:large|small|mini)['\"]" packages/vue/src/breadcrumb rg -i --glob "*.ts" "size\s*=\s*['\"](?:large|small|mini)['\"]" packages/vue/src/breadcrumb rg -i --glob "*.ts" "size:\s*['\"](?:large|small|mini)['\"]" packages/vue/src/breadcrumb echo "" echo ">> Checking in packages/vue/src/breadcrumb-item:" rg -i --glob "*.vue" "size\s*=\s*['\"](?:large|small|mini)['\"]" packages/vue/src/breadcrumb-item rg -i --glob "*.vue" "size:\s*['\"](?:large|small|mini)['\"]" packages/vue/src/breadcrumb-item rg -i --glob "*.ts" "size\s*=\s*['\"](?:large|small|mini)['\"]" packages/vue/src/breadcrumb-item rg -i --glob "*.ts" "size:\s*['\"](?:large|small|mini)['\"]" packages/vue/src/breadcrumb-item echo "Search completed."Length of output: 1348
Validation Complete: The breadcrumb size validator is consistent.
- The search in the
packages/vue/src/breadcrumbandpackages/vue/src/breadcrumb-itemdirectories confirmed that no disallowed size values ('large','small', or'mini') are in use.- The validator in
packages/vue/src/breadcrumb-item/src/index.tscorrectly mirrors the implementation in the Breadcrumb component by allowing only'medium'and an empty string.
2 participants
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #3004
What is the new behavior?
validate size property
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit