feat: support tokenize prop for custom multi-tag tokenization#1220
feat: support tokenize prop for custom multi-tag tokenization#1220ZQDesigned wants to merge 4 commits intoreact-component:masterfrom
Conversation
|
@ZQDesigned is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
演练此PR为rc-component/select添加了 变更自定义分词功能
预估代码审查工作量🎯 3 (中等) | ⏱️ ~25 分钟 可能相关的PR
建议的审查者
诗歌
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/BaseSelect/index.tsx`:
- Around line 398-408: When isCompositing is true we must avoid calling
tokenize(searchText) (and also avoid getSeparatedContent) to prevent side
effects; change the logic in the separatedList computation so that the first
branch checks isCompositing and sets separatedList = null when true, otherwise
proceed to call tokenize(searchText) and evaluate isUnchanged, and in the
fallback branch do not call getSeparatedContent when isCompositing is true.
Update the code paths that reference tokenize, getSeparatedContent,
tokenSeparators, cap, searchText and onSearchSplit so composition short-circuits
before any tokenization or split logic runs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9be0b77f-b180-42d5-b2bc-4816ac255f7f
📒 Files selected for processing (5)
docs/demo/custom-tokenize.mddocs/examples/custom-tokenize.tsxsrc/BaseSelect/index.tsxtests/Multiple.test.tsxtests/Tags.test.tsx
There was a problem hiding this comment.
Pull request overview
Adds a new tokenize prop to BaseSelect to allow custom tokenization logic (taking precedence over tokenSeparators) for tags/multiple modes, along with new demos and tests covering quote-aware splitting, IME composition behavior, and maxCount.
Changes:
- Introduce
tokenize?: (input: string) => string[]inBaseSelectand apply it in the token split path ahead oftokenSeparators. - Add tests for custom tokenization behavior in both
tagsandmultiplemodes (including composition +maxCount). - Add a new demo showcasing quote-aware tokenization.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Tags.test.tsx | Adds coverage for tokenize precedence, maxCount, composition handling, and opt-out behavior. |
| tests/Multiple.test.tsx | Adds coverage ensuring tokenized inputs in multiple only select matching options. |
| src/BaseSelect/index.tsx | Implements the tokenize prop and integrates it into search split logic. |
| docs/examples/custom-tokenize.tsx | Adds a quote-aware tokenization demo implementation. |
| docs/demo/custom-tokenize.md | Registers the new demo page in dumi. |
Comments suppressed due to low confidence (1)
src/BaseSelect/index.tsx:415
patchLabelscan benull(when composing or when no split is detected), but it is annotated asstring[]. This makes the type inaccurate and can break builds that enablestrictNullChecks(or make later refactors unsafe). Consider typing it asstring[] | null(or removing the explicit annotation and letting inference producestring[] | null).
// Check if match the `tokenSeparators` or custom `tokenize`
const patchLabels: string[] = isCompositing ? null : separatedList;
// Ignore combobox since it's not split-able
if (mode !== 'combobox' && patchLabels) {
newSearchText = '';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new tokenize prop to the BaseSelect component, allowing for custom tokenization logic that takes precedence over tokenSeparators. The changes include updated logic in BaseSelect/index.tsx to handle this new prop, along with comprehensive test cases in Multiple.test.tsx and Tags.test.tsx to verify the functionality, including quote-aware splitting and composition handling. A review comment was provided suggesting a refactor of the separatedList logic to improve readability, which is a valid improvement opportunity.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1220 +/- ##
=======================================
Coverage 99.44% 99.44%
=======================================
Files 31 31
Lines 1256 1267 +11
Branches 437 444 +7
=======================================
+ Hits 1249 1260 +11
Misses 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
tokenize?: (input: string) => string[]toBaseSelectand prioritize it overtokenSeparatorsin token split logic.maxCount, and allowtokenizeto opt out of split by returning[input].Related
Test plan
npm test -- tests/Tags.test.tsx tests/Multiple.test.tsxSummary by CodeRabbit
发布说明
新功能
文档
测试