背景
実務で使っている日付時刻操作ライブラリのMoment.jsがメンテンスモードとなったので、ライブラリ変更として推奨されている4つのうちdate-fnsに切り替えることになった。
このライブラリ変更にあたって、改修からPRのApproveを頂くまでに約1週間も掛かったてしまったため、なぜそんなに工数がかかってしまったのか、しくじり先生風にアウトプット。
※個人ブログから技術的アウトプットはQiitaへ引っ越ししたので、こちらは過去に書いたブログとなります。
初めての保守
このIssueがエンジニアとして実務に入って初めての保守業務であった。
今まで行っていた新機能開発と異なり、既存の挙動を変えないことを念頭に置く実装は初めての自分には難しかった。
先に書いておくが、多分同じ日付系のライブラリ変更をしたことがある人や、経験がある方にすれば、書いてることはすごく当たり前で、大した事ない変更だと思う。
でも残念ながらエラーにハマりにハマり、すべて基本的なところでミスと勘違いをしていた。ただ、その分学びのある時間となった。
大きく
1/ 経過時間
2/ フォーマット
3/ 出力の認識
4/ リファクタリング
この4つに分けて、
- 何をミス、勘違いしていたのか、
- どうすればこんなに工数をかけずに改修できたのか
を記載していきます。
1/ 経過時間について
これは先日の記事内容にまとめた。
抜粋しまとめると、JavaScript の経過日時を 「**秒」**と思い込んであり、意図していない経過時間が返ってきては頭を悩ませた。
正しくは、協定世界時 (UTC) からの経過「**ミリ秒」**で指定されているので、ミリ秒への変換 Timestampクラスで解決。
間違っていた原因
- JavaScript の経過日時を 「秒」と思い込んでいた点。
解決策
- 解決策としては、MDNを読めばすぐにわかった。
2/ フォーマットについて
各ライブラリに日付フォーマットはあると思うが、ここでも勘違い。
出しかかったフォーマットとして
年-月-日 時:分
例）2021-02-23 07:15
のような一般的な日時表記を出したかった。
これをMoment.jsのフォーマットに従って書くと
//Moment.js
const now = moment();
const date = now.format('YYYY-MM-DD HH:mm')
console.log('dateは：' , date)
> dateは： 2021-02-23 07:15
これを仮に同時刻でdate-fnsに書き換えた際に以下のように書き換えてしまった。
//date-fns
const now = new Date();
const date = format(now, 'yyyy-MM-dd HH-MM');
console.log('dateは：' , date)
> dateは： 2021-02-23 07:02
期待する時間は2021-02-23 07:15なので、HHまでは同じだが、分のところが違う。
Momentと比較すると13分の差。
10分後、同じコードで試してみる。
> dateは： 2021-02-23 07:02
上から10分後なので期待する時間は2021-02-23 07:25だが、今回もHHまでは同じだが、分のところが違う。出力時間は10分前と同じで、期待する時間と比較すると23分（7時25分-7時2分）のズレが起きている。
ここで、出力されている時間が固定（同じ2021-02-23 07:02が出力）されていることに気づく。
もちろん10分後にデバックをしても同じであった。
全く違う時間を出そうとしてしまっているのかなど調査し、ググりながらもなぜ時間がずれているのかもわからず、8時20分に再度確認。すると、
> dateは： 2021-02-23 08:02
1時間進んでいるではないか。
期待する時間は2021-02-23 08:20なので、ここでもやはりHHまでは同じだが、分のところが違うことに気づく。他にも色々思い当たる点を試してみたがわかりそうでわからなかった。
たまらなく先輩エンジニアにZoomで聞く。
わいのコードを確認するや直ぐに、ニヤッと笑い、アドバイス。
「yyyy-MM-dd HH-MMって声出してみ」
わい、2、3回声出して読む。声に出すわけではわからなかった。
次に先輩
ドキュメントよう読んでみ
読んだよ〜と思いながら、date-fnsのフォーマットページを確認。
分が間違ってる可能性が高いわけなので「Minute」をよく読んで見る。
何も間違っていな.....んん？？
Minute ： 「mm」
わいのコード
'yyyy-MM-dd HH-MM'
小文字の「エム」がわいのコードにはない。
ここでようやく気づく。
コードを修正し再度確認。
期待している時間は2021-02-23 08:40。
const now = new Date();
const date = format(now, 'yyyy-MM-dd HH-mm');
console.log('dateは：' , date)
> dateは： 2021-02-23 08:40
正しくでた。
間違っていた原因
- フォーマットを大文字と小文字で間違っていた
解決策
- 経過時間と同じく、公式ドキュメントをフラットに読めばすぐわかった。
- また、各ライブラリ、言語によって日時フォーマットの表記方法は異なるので、該当の公式ドキュメントを必ず参考にすること。
3/ 出力の認識
ここでは2つある。
- 認識の誤りその①：出力は改修前と「一言一句」同じく表示させよう
- 認識の誤りその②：標準時も出す必要があれば、標準時も表示させよう
認識の誤りその①：出力は改修前と「一言一句」同じく表示させよう
改修前コード
// Moment.js
const now = moment();
const date = now.format('YYYYMMDD');
console.log(date)
> 20210223
わいが書いたコード
// date-fns
const now = new Date();
const date = format(now, 'yyyy-MM-dd');
console.log(date)
> 2021-02-23
勝手に見やすいようにと年日時の各間に「-」を入れてしまった。
PRのレビューには
💡 リファクタリング時は、リファクタ前後で挙動が変わらないようにしましょう。と。当たり前だ。
今思うと当たり前なんだが、ハイフンをいれたほうが見やすいと**部分的に考えてしまい、**変えてしまった。日時フォーマットを変えるだけでは挙動も変わるものではないだろうと（完全な思い込み）。
しかし、このコードが実務では バックアップファイルの名前の一部として使われていたので、「ここが変わるとバックアップに影響がでるかもしれない」と考えるべきであった。
間違っていた原因
- フォーマットを変えても挙動は変わらないと勝手に決めつけ、改修前のコードと一部変えた
解決策
- （理由がければ） 改修前と改修後は同じ挙動を示すコードを書く
認識の誤りその②：標準時も出す必要があれば、標準時も表示させよう
改修前コード
// Moment.js
//別ファイルから関数を読み込む
import moment from '../util/moment-jst';
// ...中略...
console.log(`Target date (from: ${moment(fromTime)}, to: ${moment(toTime)})`);
> Target date (from: Fri Feb 19 2021 09:54:00 GMT+0900, to: Fri Feb 19 2021 09:59:00 GMT+0900)
- わいが書いたコード
// date-fns
import { format } from '../util/date-fns';
// ...中略...
console.log(`Target date (from: ${format(fromTime, 'E LLL dd yyyy HH:mm:ss')},
to: ${format(toTime, 'E LLL dd yyyy HH:mm:ss')})`,
);
> Target date (from: Fri Feb 19 2021 09:54:00, to: Fri Feb 19 2021 09:59:00)
よくみると下のコードには、 **タイムゾーン (GMT)**がない。
ないままPRを出すと、レビューを頂いた。
💡 タイムゾーンも出しておいてください。まあ、ここはデバックしている所なので、形式を変えるのは良い（最悪、タイムゾーンはなくてもOK）と思いますが、形式が変わっていることについて気が付いていなかった同じ形式にするのは難しいから変更したどっちだったんでしょう？
1/ 気が付いていなかったのであれば、目で見るのではなくて、新旧で出力してdiffなりとって確認しましょう。
2/ 気づいてはいたが出力が難しかったので変更したのであれば、ちゃんと変わっていることをわかるようにGithubのコメントで書きましょう
ごもっともだ。
この場合、後者の気づいてはいた。
しかし、タイムゾーンを書くべきなのか書かなくても良いか迷った挙げ句「書かなくても良いか」と謎の決めつけをした達の悪さがでてしまったことが原因。
しかし、「仮に書かなくても良いか」と思っても、気づいているならばコメントを残すべきだったと反省。
なぜなら、恐らく先輩はレビュー頂いた際にローカルでデバックをとって再現しれくているはず。
コメントを書いていれば、ローカルでわざわざ確認する手間を削減でき、「一応書いてて」とそれだけコメントを残して済んでいたと思う。
と、レビューを頂く前に考えれておくべきだった。
ちなみに正しいコードはこちら※この後、読みやすいようにフォーマットの部分を関数で切り出し、それを読み込む形に変えることで可読性が上がりました。
// date-fns
console.log(`Target date (from: ${format(fromTime, 'E LLL dd yyyy HH:mm:ss OOOO')},
to: ${format(toTime, 'E LLL dd yyyy HH:mm:ss OOOO')})`,
);
> Target date (from: Fri Feb 19 2021 09:54:00 GMT+09:00, to: Fri Feb 19 2021 09:59:00 GMT+09:00)
間違っていた原因
- タイムゾーンが改修前後で表示されていないことに気づいていたが、それを勝手に書かなくて良いと思い込み、気づいていることをコメントなしにPRを出したこと
解決策
- タイムゾーンが表記されているのであれば、そこまで記載しよう（認識の誤りその①と同じ）
- 改修前後で違いに気づいており、（意図が当て）表示しない場合、もしくは表示方法がわからなかった場合等はコメントで残す
4/ リファクタリング
これは間違いではないが、リファクタリングの基礎として学びがあったので記載。
// date-fns
// ファイル名：date-fns.ts
imort { format } from 'date-fns'
import { utcToZonedTime } from 'date-fns-tz';
export function jstString(time: Date) {
const jst = utcToZonedTime(time, 'Asia/Tokyo');
return format(jst, 'yyyy MM dd');
}
このように、タームゾーンとフォーマットまで書いてあげて、別ファイルで日時の関数が必要な際に呼び出してあげる感じで書いた。
呼び出してあげることは問題ないのだが、
これだと、
- フォーマットが上記形式以外の場合、断片的に重複した関数を増やす必要がある
- 関数名が
jstと固定文字なので、色々なタイムゾーンが出てきた時に命名と関数があってないことになる
とせっかく関数を切り出しているのに、汎用性が少なく、都度関数を増やさないといけない。
そこで、
// ファイル名：date-fns-jst.ts
import { format as formatTZ, utcToZonedTime } from 'date-fns-tz';
const tz = 'Asia/Tokyo';
export function format( date: Date | string | number, fmt: string ) : string {
return formatTZ (utcToZonedTime(date, tz), fmt , { timeZone: tz } );
}
このようにすることで、
- 呼び出し先の関数でフォーマットを自由に変えてあげれば良い（前半部分で書いた「フォーマットにハマる」のコードイメージ）
- 色々なタイムゾーンが出てきた際は、ファイルの読み込みを変更するだけで、関数の呼び出しとかは変えなくて済む
- また、ファイル名を
date-fns-jstとすることで、JSTであることを明示的にする
とした。
今日から始められるリファクタリング10選 (1) - Qiita
終えてみて
このような経緯+調査時間があり、約1週間掛かった。
終えてみてと、こんな改修にもこんだけ工数+先輩からアドバイスをもらわないとできないのかと結構ショックだった。
しかし、Approveもらう前に、レビューを頂いた先輩から「このIssueをみて最初にハマりそうと思った所全部はまってくれたw」とにっこり笑ってた。笑
むしろハマってしまったからこそ学びがあったので、今となっては良かった。
学び
- 公式ドキュメントを丁寧に読みこと
- 自分のコードを過信せずに、ドキュメントと照らし合わせよう
- 特に各ライブラリ、言語によって日時フォーマットの表記方法は異なるので、該当の公式ドキュメントを必ず参考にしよう
- 理由がなければ、出力は改修前と「一言一句」同じく表示させよう
- 既存コードの改修時に挙動・表示形式に変更を加える場合、Githubにコメントを残そう
- リファクタリング時は、なるべく重複した関数を減らすために汎用性のあるコードを書き、必要な部分で各自定義してあげよう