refactor(select-button): migrate SelectButton from Flow to TypeScript - #4757
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe PR adds typed TypeScript and Flow implementations of ChangesSelectButton component
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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/components/select-button/SelectButton.js.flow`:
- Around line 9-17: Update the Props type for SelectButton so className and
isDisabled are optional, matching their existing default values and the
supported runtime/TypeScript contract. Keep the defaults at the existing
implementation locations and change only these two property declarations.
🪄 Autofix
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 Plus
Run ID: ff5b4148-dd16-4fea-bc74-8ce55aec0ae3
⛔ Files ignored due to path filters (1)
src/components/select-button/__tests__/__snapshots__/SelectButton.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
src/components/select-button/SelectButton.js.flowsrc/components/select-button/SelectButton.stories.tsxsrc/components/select-button/SelectButton.tsxsrc/components/select-button/__tests__/SelectButton.test.tsxsrc/components/select-button/index.js.flowsrc/components/select-button/index.ts
💤 Files with no reviewable changes (1)
- src/components/select-button/SelectButton.stories.tsx
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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/components/select-button/SelectButton.js.flow`:
- Around line 9-17: Update the Props type for SelectButton so className and
isDisabled are optional, matching their existing default values and the
supported runtime/TypeScript contract. Keep the defaults at the existing
implementation locations and change only these two property declarations.
🪄 Autofix
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 Plus
Run ID: ff5b4148-dd16-4fea-bc74-8ce55aec0ae3
⛔ Files ignored due to path filters (1)
src/components/select-button/__tests__/__snapshots__/SelectButton.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
src/components/select-button/SelectButton.js.flowsrc/components/select-button/SelectButton.stories.tsxsrc/components/select-button/SelectButton.tsxsrc/components/select-button/__tests__/SelectButton.test.tsxsrc/components/select-button/index.js.flowsrc/components/select-button/index.ts
💤 Files with no reviewable changes (1)
- src/components/select-button/SelectButton.stories.tsx
🛑 Comments failed to post (1)
src/components/select-button/SelectButton.js.flow (1)
9-17: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make defaulted Flow props optional.
Lines 23 and 26 provide defaults, but Lines 11 and 14 require both properties. Flow will reject
<SelectButton>calls that omitclassNameorisDisabled, although the runtime and TypeScript contract support those calls. Mark both properties optional.Proposed fix
type Props = { children?: React.Node, - className: string, + className?: string, error?: React.Node, errorTooltipPosition?: Position, - isDisabled: boolean, + isDisabled?: boolean, /** A CSS class for the tooltip's tether element component */ tooltipTetherClassName?: string, };This conflicts with the stated Flow backward-compatibility objective.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.type Props = { children?: React.Node, className?: string, error?: React.Node, errorTooltipPosition?: Position, isDisabled?: boolean, /** A CSS class for the tooltip's tether element component */ tooltipTetherClassName?: string, };🤖 Prompt for 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. In `@src/components/select-button/SelectButton.js.flow` around lines 9 - 17, Update the Props type for SelectButton so className and isDisabled are optional, matching their existing default values and the supported runtime/TypeScript contract. Keep the defaults at the existing implementation locations and change only these two property declarations.
d9ba5ee to
dffd89f
Compare
dffd89f to
55c3838
Compare
Merge Queue Status
This pull request spent 18 seconds in the queue, including 2 seconds running CI. Required conditions to merge
|
3 participants
Convert SelectButton component to TypeScript
This PR converts
src/components/select-buttonfrom JavaScript with Flow to TypeScript.Changes
SelectButton.jstoSelectButton.tsxwith exportedSelectButtonPropsinterfaceindex.jstoindex.ts, re-exporting the component and its typesSelectButton.stories.jstoSelectButton.stories.tsx__tests__/SelectButton.test.jstoSelectButton.test.tsx.js.flowfiles for backward compatibilityTesting
src/components/select-button; all 4 pass with regenerated snapshots matching previous outputyarn lint:tsandflow checkpassComponents/SelectButton) that behavior is unchangedSummary by CodeRabbit