Conversation
Update TypeScript from 5.9.3 to 6.0.1-rc with type fixes for combobox components. Extracted ComboboxProps type alias for better type safety and added module declarations for CSS and fonts. Co-authored-by: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBump TypeScript to 6.0.2, introduce/export a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/form/field-combobox-multiple/index.tsx (1)
80-83:⚠️ Potential issue | 🟠 MajorSame type mismatch in
onValueChangeas the single-select variant.Similar to
FieldCombobox,rest.onValueChangeis typed to receiveTItem[]but is being called withitems?.map((i) => i.value)(an array of raw values). This inconsistency should be addressed in the same way as the single-select component.🐛 Option: Pass the full items array to match the expected type
onValueChange={(items: TItem[], event) => { field.onChange(items?.map((i) => i.value) ?? [], event); - rest.onValueChange?.(items?.map((i) => i.value) ?? [], event); + rest.onValueChange?.(items, event); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/form/field-combobox-multiple/index.tsx` around lines 80 - 83, The onValueChange handler in FieldComboboxMultiple is passing items?.map(i => i.value) to rest.onValueChange (and to field.onChange), causing a type mismatch because rest.onValueChange expects TItem[] while map produces an array of raw values; fix by passing the full items array to rest.onValueChange (i.e., rest.onValueChange?.(items, event)) and only pass the mapped values to field.onChange if field expects raw values, or alternatively update the prop typing for rest.onValueChange to accept the value array—adjust the handler around onValueChange to ensure rest.onValueChange receives TItem[] and field.onChange receives the expected value shape.
🧹 Nitpick comments (2)
package.json (1)
155-155: TypeScript 6.0.1-rc is a release candidate—evaluate stability requirements.Using an RC version means the project is exposed to potential instability and breaking changes before the stable release. Verify that using this version is intentional and aligns with the team's risk tolerance. If a stable
6.0.1release becomes available, consider upgrading.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 155, The package.json currently pins TypeScript to the release-candidate version "typescript": "6.0.1-rc"; decide whether an RC is intentional and either replace that entry with the stable published version (e.g., "typescript": "6.0.1" or the latest stable) or document/approve the RC usage in the repo policy. Update the package.json dependency value for the "typescript" key accordingly, run the package manager to refresh lockfiles (npm/yarn/pnpm install), and run the test/build scripts to verify compatibility; if you must keep the RC, add a short note in the repo (README or dev docs) explaining the rationale and risk acceptance.src/global.d.ts (1)
2-4: Consider removing the redundantdeclare module '*.css'declaration.The codebase explicitly imports
vite/clienttypes and uses CSS imports with query parameters like?url(seen insrc/routes/__root.tsx:26). The baredeclare module '*.css'insrc/global.d.tsis unnecessary sincevite/clientalready provides proper typing for CSS modules. WithnoImplicitAny: truein tsconfig and the code type-checking cleanly without errors, the bare declaration doesn't weaken type safety in practice, but it's redundant. Remove it unless there's a specific reason it was added.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/global.d.ts` around lines 2 - 4, Remove the redundant top-level CSS module declaration: delete the `declare module '*.css';` entry from src/global.d.ts so that the project relies on the CSS typings provided by the imported `vite/client` types; keep the other declarations (`declare module '@fontsource/*' {}` and `declare module '@fontsource-variable/*' {}`) intact and ensure no other files rely on the bare `*.css` declaration before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/form/field-combobox/index.tsx`:
- Around line 70-73: The onValueChange handler is forwarding only item?.value to
rest.onValueChange but rest.onValueChange expects the full TItem | null (per
ComboboxProps<TItem>), causing a type mismatch; update the handler so
field.onChange still receives item?.value ?? null while rest.onValueChange is
called with the full item and event (i.e. rest.onValueChange?.(item, event));
alternatively, if you intend consumers not to receive the full item, remove
onValueChange from the component props type instead of passing a value-only
argument.
In `@src/global.d.ts`:
- Line 1: Update the top-of-file comment to match the actual filename: replace
the incorrect reference "globals.d.ts" with "global.d.ts" in the comment at the
start of the file (the comment currently reads "// ./src/globals.d.ts"); this
ensures the header comment in src/global.d.ts correctly reflects the file name.
---
Outside diff comments:
In `@src/components/form/field-combobox-multiple/index.tsx`:
- Around line 80-83: The onValueChange handler in FieldComboboxMultiple is
passing items?.map(i => i.value) to rest.onValueChange (and to field.onChange),
causing a type mismatch because rest.onValueChange expects TItem[] while map
produces an array of raw values; fix by passing the full items array to
rest.onValueChange (i.e., rest.onValueChange?.(items, event)) and only pass the
mapped values to field.onChange if field expects raw values, or alternatively
update the prop typing for rest.onValueChange to accept the value array—adjust
the handler around onValueChange to ensure rest.onValueChange receives TItem[]
and field.onChange receives the expected value shape.
---
Nitpick comments:
In `@package.json`:
- Line 155: The package.json currently pins TypeScript to the release-candidate
version "typescript": "6.0.1-rc"; decide whether an RC is intentional and either
replace that entry with the stable published version (e.g., "typescript":
"6.0.1" or the latest stable) or document/approve the RC usage in the repo
policy. Update the package.json dependency value for the "typescript" key
accordingly, run the package manager to refresh lockfiles (npm/yarn/pnpm
install), and run the test/build scripts to verify compatibility; if you must
keep the RC, add a short note in the repo (README or dev docs) explaining the
rationale and risk acceptance.
In `@src/global.d.ts`:
- Around line 2-4: Remove the redundant top-level CSS module declaration: delete
the `declare module '*.css';` entry from src/global.d.ts so that the project
relies on the CSS typings provided by the imported `vite/client` types; keep the
other declarations (`declare module '@fontsource/*' {}` and `declare module
'@fontsource-variable/*' {}`) intact and ensure no other files rely on the bare
`*.css` declaration before committing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0b406244-03d2-4ff8-975f-c5ac4e7ce258
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
package.jsonsrc/components/form/field-combobox-multiple/index.tsxsrc/components/form/field-combobox/index.tsxsrc/components/ui/combobox.tsxsrc/global.d.tstsconfig.json
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
|
yoannfleurydev
changed the title
1 participant
Summary
Test plan
Summary by CodeRabbit
Chores
Refactor