optionのvalueの型をnumberからTにする #1213
Conversation
🦋 Changeset detectedLatest commit: bb00b0d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
reg-suit detected visual differences. Check this report, and review them. 🔴 🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵 What do the circles mean?The number of circles represent the number of changed images.🔴 : Changed items, ⚪ : New items, ⚫ : Deleted items, and 🔵 Passed items How can I change the check status?If reviewers approve this PR, the reg context status will be green automatically. |
okakyo
force-pushed
the
feat/1190_select
branch
from
e2f35a6 to
9289b1a
Compare
|
|
||
| export type SearchInputOption = { | ||
| export type SearchInputOption<T = number> = { | ||
| label: string; |
There was a problem hiding this comment.
型引数の初期値numberはできればなくしたいなという気持ちがあります！
numberが初期値で指定されていると、「とりあえずnumberを突っ込んでおくか」というコードを生み出しそうな予感がしておりまして、、、 💦
valueがnumber型になっているものをoptionsに突っ込んだときに、型が自動的に SearchInputOption<number> へと切り替わったり、string型のoptionsを突っ込んだときに SearchInputOption<string> となっていればOKなので、要は型引数の初期値を気にすることはないのかなと考えています！
ただ、初期値にnumberを入れないと破壊的変更になってしまうといった問題があれば、一旦はこれでも良いかなと考えております！
| dropdownMaxHeight?: string; | ||
| onChange: (value: number | null) => void; | ||
| onChange: (value: unknown) => void; | ||
| }; |
There was a problem hiding this comment.
ここのunknownはPropsの型引数で置き換えられると思います！
|
|
||
| type Props = BaseProps & { | ||
| options: SelectBoxOption[]; | ||
| value: number | null; | ||
| type Props<T = number | null> = BaseProps & { | ||
| options: SelectBoxOption<T>[]; |
There was a problem hiding this comment.
ジェネリクスでnullableを許容するというよりは、valueやonChangeのnullableを許容するところに絞って適用するのが良いかなと思います！
（型引数の初期値を使わないのが一番良さそうですが！）
okakyo
force-pushed
the
feat/1190_select
branch
from
ca641b9 to
ed19bb4
Compare
okakyo
force-pushed
the
feat/1190_select
branch
from
ed19bb4 to
4abbb5c
Compare
okakyo
force-pushed
the
feat/1190_select
branch
from
c9e9707 to
ad55c94
Compare
okakyo
force-pushed
the
feat/1190_select
branch
from
ad55c94 to
0dec9be
Compare
okakyo
force-pushed
the
feat/1190_select
branch
from
0dec9be to
54d7532
Compare
okakyo
force-pushed
the
feat/1190_select
branch
from
138a3f9 to
a2b46ba
Compare
|
色々試行錯誤してみたのですが、Vue の generics が Props, Emits に 型を適用できたが、ref, methods などへ型を引き継ぐことはできなさそうでした。この解決方法について資料を調べてみたのですが、解決策が見つからないので解決策が見つかるまで一旦保留させてください。 |
|
@okakyo |
okakyo
force-pushed
the
feat/1190_select
branch
from
a2b46ba to
b18a4f7
Compare
| style={{ width: "100%" }} | ||
| value={option.value} | ||
| value={String(option.value)} | ||
| id={`${option.label}-${option.value}`} |
There was a problem hiding this comment.
check-box-newのvalueにはstring[]といった型も一応入りうるので、Stringでパースしてしまうとバグりそうだなという印象を受けました……！
check-box-newのvalueが受け付ける型は、string | number | readonly string[] | undefined のどれかなので、SearchPopupPanelの型引数を T extends string | number | readonly string[] | undefined とすれば型の問題は回避できそうです！
| }: Props<T>) => { | ||
| // TODO: value は number に固定 | ||
| const [activeValue, setActiveValue] = useState<T | null>(null); |
There was a problem hiding this comment.
TODOのコメントアウト残っていますが消しても大丈夫なものですかね……？
|
|
||
| // FIXME: value の default 値が -1 なので number 型に固定 | ||
| return shouldShowAddButton |
There was a problem hiding this comment.
-1となっているのはaddableが有効になっているときに表示されるボタンだと思うのですが、そのボタンに当てられるvalueは使うことのない値なので、 適当に0とかの値を割り当てて、 型アサーションを使ってTで型を上書きしてしまってもよいかなぁと思っています、、、笑（もちろん事情をコメントアウトに残す前提）
search-selectorもジェネリクス化が望まれているコンポーネントなので、ここもnumber以外に対応できるとめちゃくちゃありがたいです……！ 🙇 🙇 🙇
（24/4/1 追記）
0を突っ込むと、value = 0の要素が選択されたときにバグる説がありそうですね……
追加ボタンのvalueに、options内のvalueが合致するものがなければOKなので、uidのような値を突っ込んで型アサーションで T としてしまうのがよいかなと思いました！
| <WizSelectBox | ||
| options={statusOptions} | ||
| value={status} | ||
| onChange={(selectedValue) => onChangeStatus?.(selectedValue)} | ||
| onChange={(selectedValue) => | ||
| onChangeStatus?.(selectedValue as T) | ||
| } | ||
| placeholder={statusPlaceholder} | ||
| expand | ||
| /> |
There was a problem hiding this comment.
ここの型アサーションは回避できそうでして、例えば、
| <WizSelectBox | |
| options={statusOptions} | |
| value={status} | |
| onChange={(selectedValue) => onChangeStatus?.(selectedValue)} | |
| onChange={(selectedValue) => | |
| onChangeStatus?.(selectedValue as T) | |
| } | |
| placeholder={statusPlaceholder} | |
| expand | |
| /> | |
| <WizSelectBox<T> | |
| options={statusOptions} | |
| value={status} | |
| onChange={(selectedValue) => onChangeStatus?.(selectedValue)} | |
| placeholder={statusPlaceholder} | |
| expand | |
| /> |
として、こうするとstatusOptionsの型エラーが出るので、今度はPropsのstatusOptionsを、
type Props<T> = BaseProps & {
// 中略
statusOptions?: ComponentProps<typeof WizSelectBox<T>>["options"];
// 中略
};とすれば、型アサーションなしで実装できると思います！
okakyo
force-pushed
the
feat/1190_select
branch
from
8fded01 to
bb00b0d
Compare
タスク
resolve: #1190
行ったこと
value = -1としている場合は number 型のままにしたTODO
genericsを利用して Vue.js の型定義を投げるようになったが、computed, methods の引数に型を渡せなかったため、別PR で対応したhttps://ja.vuejs.org/api/sfc-script-setup#generics