本当にtoStringで良いのかを疑う
実際にあった事例
元々の実装
code:entities.ts
// client内で使う用の型
type Item = {
price: number;
}
code:repositories.ts
// backendとのinterfaceとなっている型
// backendが受け付ける型がstringなので、stringである
type ReqItem = {
price: string;
}
// repository内で、clientのものからbackendのものに変換する
const toReqItem = (item: Item): ReqItem => {
return {
price: item.price.toString()
}
}
後に、priceに範囲の仕様が追加され、Entityの型が変わった
以下のように修正された
code:entities.ts
// client内の型
type Item = {
price: number | {max: number, min: number}
}
この際に、toReqItem内で型エラーが生じない
code:ts
const toReqItem = (item: Item): ReqItem => {
return {
price: item.price.toString() // ok
}
}
item.price.toString()の中身が、"{max: 100, min: 80}"のような文字列だとしても型は適合しているので型エラーが生じない
しかし、本当は型エラーになってほしい
上の話のどこが問題か？
server側の型がstringなのがおかしい
それはそうだが、それはclientでは対応できない、という前提の話なので今回は無視
priceの型は、Tagged Unionで定義すべき？
そうかもしれないが、この事例ではこれをしたところで解決されないし、関係ない
stringという型が荒すぎるのが問題
もっと正確に言うと、ReqItem.priceの型定義が間違っているmrsekut.icon
ReqItem.priceの型はstringではないはず
stringであるなら、どんな文字列も許容されるべきである
"100"も"hoge"も"{max:100,min:80}"も許容されるべきである
しかし実際の仕様はそうではない
仕様が正しく型に表現されていないという意味で、型が間違っている
安易な対策
こういう関数を定義すればいい
code:ts
const numberToString = (num: number): string => {
return num.toString();
};
そしてこう修正する
code:ts
const toReqItem = (item: Item): ReqItem => {
return {
price: numberToString(item.price) // error!
}
}
任意のobjectに対して.toString()を使っているのが問題なので、
numberのみtoStringするような関数を定義して使うようにする
これで今回のような修正で型エラーは生じるようになる
.toString()は使わない、というルールが必要になる
linterで設定できるなら良いが、そうじゃないならCode Reviewで全部チェックしないといけない
別の場所で同じ問題が生じうる
正しく対策する
newtypeのようなことをすれば良い
ReqItemのpriceをbranded typeかなにかで定義する
code:ts
type Price = Branded<string, "Price">;
const makePrice = (price: number) => price.toString() as Price;
そしてこう修正する
code:ts
type ReqItem = {
price: Price;
}
const toReqItem = (item: Item): ReqItem => {
return {
price: makePrice(item.price) // error!
}
}
あるいは、ReqItemをnewtype-tsか何かで定義する
今回のような修正で型エラーもちゃんと生じるし、
ReqItemの定義を見た時に意図が乗って可読性が上がる
ただの文字列ではなくnumberから変換できるもののみ許容する文字列なんだな
ということがコメントを書かずとも理解できる