Recreate PR #135: make Select value generic; patch bump via Cursor rule #157
Conversation
…t data attributes and search to string
|
|
WalkthroughThis change bumps the components package version and makes the Select UI component generic over its option value type, updating related props, handlers, comparisons, and runtime stringification to support non-string values. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant S as Select<T>
participant P as Parent
participant D as DOM
U->>S: Open dropdown / type
S->>S: Render options (each has value: T)
U->>S: Select option
S->>P: onValueChange(value: T)
Note over S,P: Value type preserved (generic T)
alt Create new option
U->>S: Submit new label
S->>S: onCreateOption(label) -> SelectOption<T> | Promise
S->>P: onValueChange(created.value: T)
end
S->>D: set data-value = String(option.value)
S->>U: Display label (uses String(value) when rendering)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
|
📝 Storybook Preview: View Storybook This preview will be updated automatically when you push new changes to this PR.
|
|
🔍 Broken test auto-fixer • Learn more
|
The generic Select changes broke placeholder display when form values are initialized with empty strings. The condition 'value != null' returns true for empty strings, causing String(value) to display instead of the placeholder. Fixed by checking both null/undefined AND empty string conditions. Fixes failing test in select.stories.tsx that expected placeholder text to be displayed initially.
|
✅ Fixed the failing test on PR #157! Issue IdentifiedThe test was failing because the generic Select component changes broke placeholder display when form values are initialized with empty strings ( The problem was in this line: {value != null ? (selectedOption?.label ?? String(value)) : placeholder}Since empty strings are not Solution AppliedFixed the condition to check for both null/undefined AND empty strings: {value != null && value !== '' ? (selectedOption?.label ?? String(value)) : placeholder}Results
The Select component now properly displays placeholder text when values are empty strings, maintaining backward compatibility while supporting the new generic value types. 💻 View my work • 🛑 Stop • 🚫 Ban all checks • React 👍 or 👎 |
![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 (3)
packages/components/src/ui/select.tsx (3)
37-41: Generic SelectProps looks good; consider ReadonlyArray for options.Avoid accidental mutation by making options read-only.
Apply:
-export interface SelectProps<T extends React.Key = string> +export interface SelectProps<T extends React.Key = string> extends Omit<React.ButtonHTMLAttributes<HTMLButtonElement>, 'value' | 'onChange'> { - options: SelectOption<T>[]; + readonly options: ReadonlyArray<SelectOption<T>>; value?: T; onValueChange?: (value: T) => void;
136-136: Prefer explicit nullish check over!= null.To satisfy strict-equality lint rules and be explicit.
- {value != null ? (selectedOption?.label ?? String(value)) : placeholder} + {value !== undefined && value !== null ? (selectedOption?.label ?? String(value)) : placeholder}
161-169: Prevent SearchInput props from overriding internal controlled state.Spreading
searchInputPropsafteronValueChangeandvalueallows consumers to overwrite them, breaking search state updates.Apply:
- <SearchInput - placeholder="Search..." - value={searchQuery} - onValueChange={(v: string) => { - setSearchQuery(v); - searchInputProps?.onValueChange?.(v); - }} - {...searchInputProps} - /> + <SearchInput + placeholder="Search..." + {...searchInputProps} + value={searchQuery} + onValueChange={(v: string) => { + setSearchQuery(v); + searchInputProps?.onValueChange?.(v); + }} + />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/components/package.json(1 hunks)packages/components/src/ui/select.tsx(9 hunks)
🧰 Additional context used
📓 Path-based instructions (18)
{package.json,packages/**/package.json,apps/**/package.json}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Use consistent versioning across packages
Files:
packages/components/package.json
{packages/**/package.json,apps/**/package.json}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Define peerDependencies, dependencies, and devDependencies appropriately in package.json
Files:
packages/components/package.json
packages/components/package.json
📄 CodeRabbit inference engine (.cursor/rules/versioning-with-npm.mdc)
Version bumps for @lambdacurry/forms update packages/components/package.json
Files:
packages/components/package.json
packages/**/package.json
📄 CodeRabbit inference engine (AGENTS.md)
Verify each package’s exports field is correct
Files:
packages/components/package.json
packages/components/src/ui/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
packages/components/src/ui/**/*.tsx: Build on @radix-ui components as the foundation for base UI components
Follow the component composition pattern for UI components that accept form integration
Files:
packages/components/src/ui/select.tsx
packages/components/src/ui/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Base UI components should be named as ComponentName in ui/ directory
Files:
packages/components/src/ui/select.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
**/*.{tsx,ts}: Props interfaces should be named as ComponentNameProps
Form schemas should be named formSchema or componentNameSchema
Files:
packages/components/src/ui/select.tsx
packages/components/src/{remix-hook-form,ui}/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Always export both the component and its props type
Files:
packages/components/src/ui/select.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
{apps,packages}/**/*.{ts,tsx}: Use package name imports for published packages (e.g., import { TextField } from '@lambdacurry/forms/remix-hook-form')
Import from specific entry points (e.g., import { TextField } from '@lambdacurry/forms/remix-hook-form/text-field')
Do not use relative imports across packages (e.g., avoid import { TextField } from '../../packages/components/src/remix-hook-form/text-field')
Order imports: 1) external libraries, 2) internal package imports, 3) cross-package imports, 4) type-only imports (grouped separately)
Files:
packages/components/src/ui/select.tsx
{apps,packages}/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
{apps,packages}/**/src/**/*.{ts,tsx}: Use relative imports within the same package (e.g., import { FormControl } from './form')
Use relative imports for sibling directories (e.g., import { Button } from '../ui/button')
Files:
packages/components/src/ui/select.tsx
packages/components/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
packages/components/src/**/*.{ts,tsx}: Always export both component and props type (e.g., export { ComponentName }; export type { ComponentNameProps };)
Use named exports for components for better tree-shaking (e.g., export const ComponentName = ...; avoid default exports)
Avoid default exports for components
Use tree-shaking friendly exports
Files:
packages/components/src/ui/select.tsx
{apps,packages}/**/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
{apps,packages}/**/src/**/*.{tsx,ts}: Use kebab-case for component file names (e.g., text-field.tsx, data-table.tsx)
Match component name in PascalCase to the exported component from the file (e.g., text-field.tsx exports TextField)
Files:
packages/components/src/ui/select.tsx
packages/components/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Don't import from remix-hook-form package in ui components
Files:
packages/components/src/ui/select.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Avoid circular dependencies
**/*.{ts,tsx}: Indent with 2 spaces, max line width 120, and use single quotes (Biome-enforced)
Use PascalCase for component and type names
Name hooks in camelCase with a use* prefix
Organize imports automatically with Biome
Use named exports only
Files:
packages/components/src/ui/select.tsx
packages/components/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all @lambdacurry/forms source under packages/components/src and treat it as the only editable source for the package
Files:
packages/components/src/ui/select.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Keep React components pure and fully typed
Define explicit props interfaces for React components
Use React 19 ref patterns (forwardRef, useImperativeHandle as appropriate)
Files:
packages/components/src/ui/select.tsx
**/*.{ts,tsx,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use kebab-case for filenames (e.g., text-field.tsx, data-table-filter/**)
Files:
packages/components/src/ui/select.tsx
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid cross-package relative imports in the monorepo
Files:
packages/components/src/ui/select.tsx
🧬 Code graph analysis (1)
packages/components/src/ui/select.tsx (1)
packages/components/src/remix-hook-form/select.tsx (2)
SelectProps(7-20)Select(22-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (7)
packages/components/src/ui/select.tsx (6)
20-23: Generic SelectOption over React.Key is appropriate.Good constraint for DOM/React key safety while enabling number/string values.
53-56: onCreateOption returning SelectOption aligns with the generic model.This preserves type safety through creation.
64-82: Generic component signature with default = string preserves source compatibility.API surface is typed correctly and remains non-breaking for existing callers.
175-181: Stringifying value for data- and test id is correct.*Prevents non-string dataset issues and stabilizes test selectors.
243-245: Exact-match gate accounts for label and value.This avoids duplicate creation when the typed value matches an existing option.
254-257: Creation flow returns the typed value.Correctly propagates created.value of type T.
packages/components/package.json (1)
3-3: Patch bump to 0.22.3 looks correct — confirm no exact 0.22.2 pins remaincomponents version = 0.22.3; sandbox scan was inconclusive. Run these locally to confirm no consumers pin @lambdacurry/forms to 0.22.2:
rg -n --hidden --no-ignore '"@lambdacurry/forms"\s*:\s*".+?"' -S || echo "None found"
rg -n --hidden --no-ignore '"@lambdacurry/forms"\s*:\s*"0.22.2"' -S || echo "None found"
rg -n --hidden --no-ignore '@lambdacurry/forms[^0-9]*0.22.2' -S || echo "None found"
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/components/src/ui/select.tsx(9 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
packages/components/src/ui/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
packages/components/src/ui/**/*.tsx: Build on @radix-ui components as the foundation for base UI components
Follow the component composition pattern for UI components that accept form integration
Files:
packages/components/src/ui/select.tsx
packages/components/src/ui/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Base UI components should be named as ComponentName in ui/ directory
Files:
packages/components/src/ui/select.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
**/*.{tsx,ts}: Props interfaces should be named as ComponentNameProps
Form schemas should be named formSchema or componentNameSchema
Files:
packages/components/src/ui/select.tsx
packages/components/src/{remix-hook-form,ui}/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Always export both the component and its props type
Files:
packages/components/src/ui/select.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
{apps,packages}/**/*.{ts,tsx}: Use package name imports for published packages (e.g., import { TextField } from '@lambdacurry/forms/remix-hook-form')
Import from specific entry points (e.g., import { TextField } from '@lambdacurry/forms/remix-hook-form/text-field')
Do not use relative imports across packages (e.g., avoid import { TextField } from '../../packages/components/src/remix-hook-form/text-field')
Order imports: 1) external libraries, 2) internal package imports, 3) cross-package imports, 4) type-only imports (grouped separately)
Files:
packages/components/src/ui/select.tsx
{apps,packages}/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
{apps,packages}/**/src/**/*.{ts,tsx}: Use relative imports within the same package (e.g., import { FormControl } from './form')
Use relative imports for sibling directories (e.g., import { Button } from '../ui/button')
Files:
packages/components/src/ui/select.tsx
packages/components/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
packages/components/src/**/*.{ts,tsx}: Always export both component and props type (e.g., export { ComponentName }; export type { ComponentNameProps };)
Use named exports for components for better tree-shaking (e.g., export const ComponentName = ...; avoid default exports)
Avoid default exports for components
Use tree-shaking friendly exports
Files:
packages/components/src/ui/select.tsx
{apps,packages}/**/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
{apps,packages}/**/src/**/*.{tsx,ts}: Use kebab-case for component file names (e.g., text-field.tsx, data-table.tsx)
Match component name in PascalCase to the exported component from the file (e.g., text-field.tsx exports TextField)
Files:
packages/components/src/ui/select.tsx
packages/components/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Don't import from remix-hook-form package in ui components
Files:
packages/components/src/ui/select.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Avoid circular dependencies
**/*.{ts,tsx}: Indent with 2 spaces, max line width 120, and use single quotes (Biome-enforced)
Use PascalCase for component and type names
Name hooks in camelCase with a use* prefix
Organize imports automatically with Biome
Use named exports only
Files:
packages/components/src/ui/select.tsx
packages/components/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Place all @lambdacurry/forms source under packages/components/src and treat it as the only editable source for the package
Files:
packages/components/src/ui/select.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Keep React components pure and fully typed
Define explicit props interfaces for React components
Use React 19 ref patterns (forwardRef, useImperativeHandle as appropriate)
Files:
packages/components/src/ui/select.tsx
**/*.{ts,tsx,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Use kebab-case for filenames (e.g., text-field.tsx, data-table-filter/**)
Files:
packages/components/src/ui/select.tsx
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid cross-package relative imports in the monorepo
Files:
packages/components/src/ui/select.tsx
🧬 Code graph analysis (1)
packages/components/src/ui/select.tsx (1)
packages/components/src/remix-hook-form/select.tsx (2)
SelectProps(7-20)Select(22-52)
🔇 Additional comments (7)
packages/components/src/ui/select.tsx (7)
20-23: Generic SelectOption typed to React.Key is solidConstraint to React.Key keeps keys/DOM-safe and aligns with React element keys. No issues.
54-54: onCreateOption generic return type looks correctThe union with Promise maintains flexibility for async create flows.
64-81: Generic Select component signature: OKDefault generic preserves existing usage; inference from options/value will work in TSX.
179-181: Stringifying values for data- and test ids: good*Ensures stable attributes across number/string values and keeps DOM compliant.
256-257: Created option piping its value to onValueChange is correctMatches the new generic typing and expected controlled Select behavior.
136-136: Treat only null/undefined as “no selection” (allow empty-string values)Empty-string values are currently treated as “no selection” and their labels won’t display; change the check to only exclude null/undefined:
- {value != null && value !== '' ? (selectedOption?.label ?? String(value)) : placeholder} + {value !== undefined && value !== null ? (selectedOption?.label ?? String(value)) : placeholder}If '' is intentionally used to mean “no selection”, keep as-is and document it.
37-41: Generic SelectProps correctly propagates typed value/onValueChangeOmitting HTML 'value'/'onChange' avoids collisions; generic typing with default = string is sound and backward-compatible. Repo search for UISelect/Select returned no matches — verify downstream wrappers/external consumers still type-check and update call sites to specify the generic if they rely on the string default.
| options.some((o) => o.label.toLowerCase() === lower || String(o.value).toLowerCase() === lower); | ||
| if (!creatable || !q || hasExactMatch) return null; |
There was a problem hiding this comment.
Creatable exact-match can suppress “Create” while showing no results
You added exact-match against String(o.value), but CommandItem search still uses only label, so typing a value that matches an existing option’s value (but not its label) produces “No results” and also hides the create option.
Two fixes (pick one):
- Include value in the item’s search string:
- Change each CommandItem value prop to include both: value={
${option.label} ${String(option.value)}}
- Change each CommandItem value prop to include both: value={
- If supported by your Command/CommandItem, add keywords with the stringified value:
- keywords={[String(option.value)]}
Example (outside the changed hunk):
// For both branches where CommandItem is rendered
<CommandItem
...
- value={option.label}
+ value={`${option.label} ${String(option.value)}`}
...
>This aligns the exact-match guard with the visible search results and avoids a confusing “No results” state.
🤖 Prompt for AI Agents
In packages/components/src/ui/select.tsx around lines 244-245, the exact-match
check compares the query to String(o.value) but CommandItem search only uses the
label, causing a hidden "Create" when a value (not label) matches; fix by making
CommandItem searchable on both label and value or by adding keywords: either set
each CommandItem value to include both label and stringified value
(value={`${option.label} ${String(option.value)}`}) or, if supported, add
keywords={[String(option.value)]} to the CommandItem props so the visible search
results and the creatable exact-match guard stay in sync.
Requested by Jake Ruesink.
This recreates the changes from PR #135 against current main:
Versioning (Cursor rule):
Notes:
Related: #135
💻 View my work • About Codegen
⛔ Remove Codegen from PR • 🚫 Ban action checks
Summary by CodeRabbit
New Features
Chores