♻️ refactor: 全コンポーネントを React 19 の ref-as-prop に移行（塩漬け）#274
Conversation
- `React.forwardRef` を廃止し、ref を通常の prop として受け取る React 19 流に書き換え - これにより Radix `Trigger asChild` 等から Button へ直接 ref を渡せるようになり、`sparkle-design-internal#182` の `PopoverTriggerButton` workaround が削除可能になる - 前提として `@types/react`/`@types/react-dom` および `react`/`react-dom` を `^19` にバンプ（ref-as-prop は React 19 ランタイム機能のため） - `ButtonProps` の `NativeButtonProps` を `React.ComponentPropsWithoutRef` → `React.ComponentProps` に変更 - `IconButtonProps` の base 型を `React.ButtonHTMLAttributes` → `React.ComponentProps<"button">` に変更 refs: #270 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough多数の UI コンポーネントで props の基底型を React.ComponentProps 系に統一し、forwardRef ベースの実装を通常の関数コンポーネントに置換して ref を props から受け渡すようにした一括リファクタ。 ChangesforwardRef 廃止と ComponentProps への統一
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31a145c2cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| autoInstallPeers: true | ||
| excludeLinksFromLockfile: false |
There was a problem hiding this comment.
With this lockfile, a clean CI/dev install using the normal frozen path fails before dependencies are installed because package.json still defines pnpm.overrides but the regenerated lockfile no longer records them; I verified pnpm install --frozen-lockfile exits with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH. Please regenerate the lockfile with the repository's current pnpm configuration so the overrides block matches package.json.
Useful? React with 👍 / 👎.
![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/ui/icon-button/index.tsx`:
- Around line 389-420: The IconButton currently ignores the children prop when
asChild is true and always renders the internal Icon/Spinner and unconditional
attributes (type="button", disabled), breaking composition and semantics; update
the return logic in the IconButton component so that when asChild is true (Comp
= SlotPrimitive.Slot) you render children instead of the internal Icon/Spinner
and ensure ref and className are composed onto the child via Slot, and only
apply type="button", disabled and button-specific event suppression (e.g.,
prevent clicks when disabled) when Comp is the native "button"; keep using
iconButtonVariants, isIconButtonDisabled and props spreading for other
attributes but conditionally omit button-only attributes when asChild is used.
🪄 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: bbb8035f-40f7-46be-86e8-b48b8a7af9ef
⛔ Files ignored due to path filters (2)
package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (2)
src/components/ui/button/index.tsxsrc/components/ui/icon-button/index.tsx
| const Comp = asChild ? SlotPrimitive.Slot : "button"; | ||
|
|
||
| // アイコンのサイズをボタンサイズに合わせて設定 | ||
| const getIconSize = () => { | ||
| switch (size) { | ||
| case "xs": | ||
| return 3; | ||
| case "sm": | ||
| return 5; | ||
| case "lg": | ||
| return 7; | ||
| default: | ||
| return 6; | ||
| } | ||
| }; | ||
| // アイコンのサイズをボタンサイズに合わせて設定 | ||
| const getIconSize = () => { | ||
| switch (size) { | ||
| case "xs": | ||
| return 3; | ||
| case "sm": | ||
| return 5; | ||
| case "lg": | ||
| return 7; | ||
| default: | ||
| return 6; | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <Comp | ||
| type="button" | ||
| className={cn( | ||
| iconButtonVariants({ | ||
| variant, | ||
| size, | ||
| theme, | ||
| isLoading, | ||
| isDisabled, | ||
| className, | ||
| }) | ||
| )} | ||
| ref={ref} | ||
| disabled={isIconButtonDisabled} | ||
| {...props} | ||
| > | ||
| {isLoading ? ( | ||
| <> | ||
| <Spinner size={getIconSize()} className="text-current" /> | ||
| </> | ||
| ) : ( | ||
| <Icon icon={icon} size={getIconSize()} /> | ||
| )} | ||
| </Comp> | ||
| ); | ||
| } | ||
| ); | ||
| return ( | ||
| <Comp | ||
| ref={ref} | ||
| type="button" | ||
| className={cn( | ||
| iconButtonVariants({ | ||
| variant, | ||
| size, | ||
| theme, | ||
| isLoading, | ||
| isDisabled, | ||
| className, | ||
| }) | ||
| )} | ||
| disabled={isIconButtonDisabled} | ||
| {...props} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l src/components/ui/icon-button/index.tsxRepository: goodpatch/sparkle-design
Length of output: 112
🏁 Script executed:
cat -n src/components/ui/icon-button/index.tsx | sed -n '290,310p'Repository: goodpatch/sparkle-design
Length of output: 686
🏁 Script executed:
cat -n src/components/ui/icon-button/index.tsx | sed -n '355,435p'Repository: goodpatch/sparkle-design
Length of output: 2347
asChild の合成が成立していません。
children プロップは分割代入されていますが（line 372）、return 内で一度も描画されていません。代わりに <Icon> / <Spinner> のみが常にレンダリングされるため、<IconButton asChild><a href="..." /></IconButton> のような使用時に、caller が提供した要素は DOM に含まれず、ref や className も合成されません。また type="button" と disabled も無条件に Comp へ渡されるため、非 button 要素とのスロット合成ではセマンティクスが崩れます。Button コンポーネントと同様に asChild 時は children を描画する分岐を追加し、type・disabled・イベント抑止を条件分けしてください。
修正イメージ
return (
<Comp
ref={ref}
- type="button"
+ type={asChild ? undefined : "button"}
+ aria-busy={isLoading || undefined}
+ aria-disabled={asChild && isIconButtonDisabled ? true : undefined}
className={cn(
iconButtonVariants({
variant,
size,
theme,
isLoading,
isDisabled,
className,
})
)}
- disabled={isIconButtonDisabled}
+ disabled={asChild ? undefined : isIconButtonDisabled}
{...props}
>
- {isLoading ? (
+ {asChild ? (
+ children
+ ) : isLoading ? (
<>
<Spinner size={getIconSize()} className="text-current" />
</>
) : (
<Icon icon={icon} size={getIconSize()} />
)}
</Comp>
);🤖 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/ui/icon-button/index.tsx` around lines 389 - 420, The
IconButton currently ignores the children prop when asChild is true and always
renders the internal Icon/Spinner and unconditional attributes (type="button",
disabled), breaking composition and semantics; update the return logic in the
IconButton component so that when asChild is true (Comp = SlotPrimitive.Slot)
you render children instead of the internal Icon/Spinner and ensure ref and
className are composed onto the child via Slot, and only apply type="button",
disabled and button-specific event suppression (e.g., prevent clicks when
disabled) when Comp is the native "button"; keep using iconButtonVariants,
isIconButtonDisabled and props spreading for other attributes but conditionally
omit button-only attributes when asChild is used.
There was a problem hiding this comment.
Pull request overview
React 19 の ref-as-prop パターンへ移行することで、Button / IconButton を Radix の asChild 合成などでより自然に ref 連携できるようにしつつ、React/型定義を React 19 系に揃える PR です。
Changes:
Buttonの props 型をReact.ComponentProps<"button">ベースに変更し、refを props として受け取って内部要素へ渡すように更新IconButtonをReact.forwardRefから ref-as-prop の関数コンポーネントへ移行react/react-dom/@types/react/@types/react-domを^19に更新
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/components/ui/button/index.tsx | ref を props で受けて <Comp> に渡す形へ型と実装を更新 |
| src/components/ui/icon-button/index.tsx | forwardRef を廃止し ref-as-prop 化（ただし asChild の子要素取り扱いに不具合あり） |
| package.json | React 本体・型定義を ^19 にバンプし、React 18 サポートを終了 |
| disabled, | ||
| icon, | ||
| ref, | ||
| children, | ||
| ...props |
- Card 系（ClickableCard / Card / CardHeader / CardTitle / CardDescription / CardControl / CardContent / CardFooter） - Input / Textarea / InputPassword - Spinner / Icon - Link / Divider - InlineMessage / InlineMessageTitle / InlineMessageDescription - Overlay / Tag / Tabs (TabsTrigger) すべての `React.forwardRef` を廃止し、ref を通常の prop として受け取る形式に変換。 基底型も `React.HTMLAttributes<*>` / `React.*HTMLAttributes<*>` から `React.ComponentProps<"*">` に統一して、ref を含む形に揃えた。 これで sparkle-design 全体が shadcn/ui 流派の React 19 ref-as-prop パターンに揃う。 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pnpm 11 で `pnpm.overrides` フィールドが package.json から読まれなくなったため、 推奨形式である pnpm-workspace.yaml の overrides フィールドに移行。 これにより ローカル pnpm 11 と Vercel pnpm 10 の間で lockfile 整合性が取れず、 `ERR_PNPM_LOCKFILE_CONFIG_MISMATCH` で CI が落ちていた問題を解消する。 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
touyou
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/components/ui/input-password/index.tsx (1)
35-42: 💤 Low valueインラインコメントに英語訳が不足しています
コーディングガイドラインでは、インラインコメントは「日本語の後に
en:プレフィックス付きで英語訳を記載する」形式が求められています。例:
- // パスワードの表示状態を管理 + // パスワードの表示状態を管理 en: Manage password visibility state const [isVisibility, setIsVisibility] = React.useState(false); - // パスワード表示切り替えハンドラー + // パスワード表示切り替えハンドラー en: Password visibility toggle handler const handleTogglePasswordVisibility = React.useCallback(() => {As per coding guidelines: "Write inline code comments in the format: Japanese description followed by
en:prefix and English translation"🤖 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/ui/input-password/index.tsx` around lines 35 - 42, Update the inline comments inside the InputPassword component to include English translations using the required format: keep the existing Japanese comment and append an English translation prefixed with "en:" for each comment (e.g., the comment above the isVisibility state and the comment above handleTogglePasswordVisibility). Locate the comments in the function InputPassword and adjust them to follow the guideline "Japanese description followed by `en:` and the English translation" so both comments (for isVisibility and handleTogglePasswordVisibility) comply.src/components/ui/input/index.tsx (1)
217-328: 💤 Low valueインラインコメントに英語訳を追加することを推奨します
このファイル内の多くのインラインコメント（例: Line 217
// 状態管理、Line 221、Line 224 など）が日本語のみで記載されています。コーディングガイドラインではen:プレフィックス付きの英語訳を含める形式が求められています。例:
- // 状態管理 + // 状態管理 en: State management const [isInputFocused, setIsInputFocused] = React.useState(false);本PRの主目的はref移行のため、この修正は別PRでまとめて対応しても問題ありません。
As per coding guidelines: "Inline code comments within components must use Japanese followed by
en:prefix with English translation"🤖 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/ui/input/index.tsx` around lines 217 - 328, Many inline comments in this component are only in Japanese; update them to include an English translation prefixed with "en:" (e.g., "状態管理 // en: state management"). Locate comments around the state and handlers such as the isInputFocused/isIconButtonFocused declarations, handleChange, handleInputFocus, handleInputBlur, handleIconButtonFocus, handleIconButtonBlur, handleContainerClick, the useEffect outside click handler (handleOutsideClick), and iconButtonSize memo, and append concise English translations after the Japanese text using the `en:` prefix. You can do this in a follow-up PR if preferred per the note in the review; keep translations short and consistent with existing Japanese comments.src/components/ui/textarea/index.tsx (1)
69-69: コーディングガイドラインと React 19 パターンの不一致を確認en: Verify coding guideline vs React 19 pattern mismatch
コーディングガイドラインでは
React.ComponentPropsWithoutRef<>を拡張するよう指定されていますが、このコードではReact.ComponentProps<"textarea">(=ComponentPropsWithRef) を使用しています。en: The coding guideline specifies extending
React.ComponentPropsWithoutRef<>, but this code usesReact.ComponentProps<"textarea">(=ComponentPropsWithRef).ただし、これは React 19 の ref-as-prop パターンに必要な変更です。
ComponentPropsWithoutRefを使用すると、ref が型に含まれず、Line 103 での ref の分割代入と Line 121 での ref の受け渡しが TypeScript エラーになります。en: However, this is a necessary change for React 19's ref-as-prop pattern. Using
ComponentPropsWithoutRefwould excludereffrom the type, causing TypeScript errors at line 103 (destructuring ref) and line 121 (passing ref).推奨事項 / Recommendation: React 19 への移行に伴い、コーディングガイドラインを更新して
React.ComponentProps<>またはReact.ComponentPropsWithRef<>の使用を推奨することを検討してください。また、React.forwardRefの使用に関する記述も React 19 では非推奨になったため、更新が必要です。en: Consider updating the coding guidelines to recommend
React.ComponentProps<>orReact.ComponentPropsWithRef<>for React 19 migration. Also update the guidance aboutReact.forwardRefas it's deprecated in React 19.As per coding guidelines: "Use consistent TypeScript component interface patterns extending React.ComponentPropsWithoutRef<> and VariantProps" and "Use
React.forwardReffor proper ref forwarding in components" - noting these guidelines appear to be based on React 18 patterns and may need updating for React 19.🤖 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/ui/textarea/index.tsx` at line 69, The component's props extend React.ComponentProps<"textarea"> (which includes ref) while the project guideline demands React.ComponentPropsWithoutRef<>, causing a guideline mismatch; update either the code or the guideline: if staying on React 19 ref-as-prop pattern keep React.ComponentProps<"textarea"> or explicitly use React.ComponentPropsWithRef<"textarea"> to type the props (so the destructured ref and the ref prop you pass through compile), and update any docs about using React.forwardRef (now deprecated in React 19) to reflect the new ref-as-prop pattern; adjust the type declaration replacing ComponentPropsWithoutRef<> with ComponentProps<>/ComponentPropsWithRef<> and ensure the ref destructuring and propagation sites use the matching prop name and type.
🤖 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.
Nitpick comments:
In `@src/components/ui/input-password/index.tsx`:
- Around line 35-42: Update the inline comments inside the InputPassword
component to include English translations using the required format: keep the
existing Japanese comment and append an English translation prefixed with "en:"
for each comment (e.g., the comment above the isVisibility state and the comment
above handleTogglePasswordVisibility). Locate the comments in the function
InputPassword and adjust them to follow the guideline "Japanese description
followed by `en:` and the English translation" so both comments (for
isVisibility and handleTogglePasswordVisibility) comply.
In `@src/components/ui/input/index.tsx`:
- Around line 217-328: Many inline comments in this component are only in
Japanese; update them to include an English translation prefixed with "en:"
(e.g., "状態管理 // en: state management"). Locate comments around the state and
handlers such as the isInputFocused/isIconButtonFocused declarations,
handleChange, handleInputFocus, handleInputBlur, handleIconButtonFocus,
handleIconButtonBlur, handleContainerClick, the useEffect outside click handler
(handleOutsideClick), and iconButtonSize memo, and append concise English
translations after the Japanese text using the `en:` prefix. You can do this in
a follow-up PR if preferred per the note in the review; keep translations short
and consistent with existing Japanese comments.
In `@src/components/ui/textarea/index.tsx`:
- Line 69: The component's props extend React.ComponentProps<"textarea"> (which
includes ref) while the project guideline demands
React.ComponentPropsWithoutRef<>, causing a guideline mismatch; update either
the code or the guideline: if staying on React 19 ref-as-prop pattern keep
React.ComponentProps<"textarea"> or explicitly use
React.ComponentPropsWithRef<"textarea"> to type the props (so the destructured
ref and the ref prop you pass through compile), and update any docs about using
React.forwardRef (now deprecated in React 19) to reflect the new ref-as-prop
pattern; adjust the type declaration replacing ComponentPropsWithoutRef<> with
ComponentProps<>/ComponentPropsWithRef<> and ensure the ref destructuring and
propagation sites use the matching prop name and type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7a7209c4-1a3a-44b7-92dc-10c1e6b647be
⛔ Files ignored due to path filters (3)
package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yamlpnpm-workspace.yamlis excluded by!**/*.yaml
📒 Files selected for processing (12)
src/components/ui/card/index.tsxsrc/components/ui/divider/index.tsxsrc/components/ui/icon/index.tsxsrc/components/ui/inline-message/index.tsxsrc/components/ui/input-password/index.tsxsrc/components/ui/input/index.tsxsrc/components/ui/link/index.tsxsrc/components/ui/overlay/index.tsxsrc/components/ui/spinner/index.tsxsrc/components/ui/tabs/index.tsxsrc/components/ui/tag/index.tsxsrc/components/ui/textarea/index.tsx
Summary
React.forwardRefを撤廃し、ref-as-prop（React 19 流） に統一モチベーション
issue #270 のコメントで言及されている通り、
Buttonは既に `function Button(props) { ... }` の React 19 流の書き方になっているがforwardRefを含めずに ref を受け取れない状態。shadcn/ui は 2024 年に forwardRef を全面廃止して ref-as-prop に移行済みで、sparkle-design もその流派なので素直に揃えるのが筋。変更点
コンポーネント（全 11 ファイル）
Button/IconButtonCard系（ClickableCard/Card/CardHeader/CardTitle/CardDescription/CardControl/CardContent/CardFooter）Input/Textarea/InputPasswordSpinner/IconLink/DividerInlineMessage/InlineMessageTitle/InlineMessageDescriptionOverlay/Tag/Tabs（TabsTrigger）すべて
React.forwardRef<*, *>(({...}, ref) => ...)をfunction X({ ref, ... }: XProps)に変換。基底型もReact.HTMLAttributes<*>/React.*HTMLAttributes<*>からReact.ComponentProps<\"*\">に統一（ref を含む形）。依存
react: `^18 || ^19` → `^19`react-dom: `^18 || ^19` → `^19`@types/react: `^18` → `^19`@types/react-dom: `^18` → `^19`CI 修正
pnpm.overrides（package.json）をpnpm-workspace.yamlのoverrides:に移行。pnpm 11 でpnpm.overridesフィールドが読まれなくなった影響で Vercel pnpm 10 と lockfile 不整合が発生していた問題を解消。ref-as-prop は React 19 ランタイム機能のため、React 18 では
refprop が剥がされて機能しません（試したところ ref forwarding テストが React 18 で落ちることを確認済み）。そのため
reactの対応バージョンを `^19` 以上に絞っており、React 18 を使い続けている consumer はこの PR をマージすると ref forwarding が無言で壊れます。そこで本 PR は次の運用を想定しています：
issue #270 について
別途 main 基点で forwardRef で Button をラップするだけの小さい PR を切ります。そちらで issue #270 を React 18 互換のまま解決します。
Test plan
tsc --noEmitクリーンunitプロジェクト全 595 件 passpnpm install --frozen-lockfile通るrefを直接渡せることを確認🤖 Generated with Claude Code