Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: useId 関数を削除する #4926

Merged
merged 1 commit into from
Sep 19, 2024
Merged

chore: useId 関数を削除する #4926

merged 1 commit into from
Sep 19, 2024

Conversation

s-sasaki-0529
Copy link
Contributor

@s-sasaki-0529 s-sasaki-0529 commented Sep 17, 2024

関連URL

概要

前回PRでは独自実装の useId の部分から、React 17 互換のための機能を廃止し、React 18 未満のサポートを終了した。

本PRでは残作業として、 useId 自体を削除し、利用しているコンポーネント側に責務を移動した。

変更内容

  • 独自実装の useId を削除
  • ↑を使用している箇所を、React 組み込みの useId を使用するように差し替え
  • 独自実装で提供していた機能を使用していた箇所のコンポーネント側の調整

確認方法

useId を使用しているコンポーネントにて、id が適切に割り振られていることが確認できればOKです。

いくつかのコンポーネントで、デフォ値を指定した場合、しない場合それぞれで id が意図通りに設定されていることは確認しました。(こういうのは単体テストでカバーしたいので整理していきたい)

備考

独自実装の useId は内部向けの Hooks で、パッケージ外に公開していない (index から export していない) ため、破壊的変更扱いにはせず、chore としています。

Copy link

pkg-pr-new bot commented Sep 17, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/kufu/smarthr-ui@4926

commit: 8d8c962

@@ -119,7 +119,8 @@ export const CheckBox = forwardRef<HTMLInputElement, Props>(
}
}, [checked, mixed])

const checkBoxId = useId(props.id)
const defaultId = useId()
const checkBoxId = props.id || defaultId
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hooks は条件分岐の中に入れることができないので

checkBoxId = props.id || useId()

と書くことはできません。
ので条件の外で一時変数が必要になるんですが、これを考えると独自実装の useId も便利だったなぁという気はちょっとしました。

import { tv } from 'tailwind-variants'

import { useId } from '../../hooks/useId'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

独自実装の useId を使用している箇所を、すべて React 組み込みの方を参照するように修正してます。

@s-sasaki-0529 s-sasaki-0529 marked this pull request as ready for review September 17, 2024 06:19
@s-sasaki-0529 s-sasaki-0529 requested a review from a team as a code owner September 17, 2024 06:19
@s-sasaki-0529 s-sasaki-0529 requested review from moshisora and nabeliwo and removed request for a team September 17, 2024 06:19
@s-sasaki-0529 s-sasaki-0529 changed the title chore: 独自実装の useId を削除する chore: useId 関数を削除する Sep 17, 2024
Copy link
Member

@nabeliwo nabeliwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@s-sasaki-0529 s-sasaki-0529 self-assigned this Sep 19, 2024
@s-sasaki-0529 s-sasaki-0529 merged commit 57ba6b3 into master Sep 19, 2024
13 checks passed
@s-sasaki-0529 s-sasaki-0529 deleted the remove-use-id branch September 19, 2024 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants