search

Tokyo-Metro-Gov / covid19

東京都 新型コロナウイルス感染症対策サイト / Tokyo COVID-19 Task Force website
https://stopcovid19.metro.tokyo.lg.jp/
MIT License
6.26k stars 1.97k forks source link

chart.jsが依存しているmoment.jsをday.jsに置き換える #2079

Closed t-tajiri closed 4 years ago

t-tajiri commented 4 years ago

改善詳細 / Details of Improvement

スクリーンショット / Screenshot

期待する見せ方・挙動 / Expected behavior

  1. yarn build --analyzeにて解析結果を確認したときに、moment.jsが存在しないこと

    after

  2. 修正前のチャートの表示と相違がないこと

動作環境・ブラウザ / Environment

t-tajiri commented 4 years ago

作業します！

t-tajiri commented 4 years ago

下のissueに影響しますね。。。

https://github.com/tokyo-metropolitan-gov/covid19/issues/2052 Month displayed in Japanese on graph x-axis on all language https://github.com/tokyo-metropolitan-gov/covid19/issues/1966 実行時に、Deprecation warning(value provided is not in a recognized RFC2822 or ISO format)が発生する

MaySoMusician commented 4 years ago

2054 でグラフX軸部分については moment dayjs ともに使用しない案が提案されました

いずれにせよ chart.js 内の moment.js は使用せずに済むかなと思います

t-tajiri commented 4 years ago

@MaySoMusician https://github.com/tokyo-metropolitan-gov/covid19/pull/2054 の修正はmoment.jsに渡すためのデータの形式をM月からnativeのDateに変更してるように見えますね。。。

MaySoMusician commented 4 years ago

このプロジェクトで使われてるchart.jsは（少なくともNode.js v10.19.0だと）v2.9.3で、またadapterの分離が終わってないみたいですね これって上書きできるのだろうか...

t-tajiri commented 4 years ago

@MaySoMusician ちょうど今完成したので、表示に問題ないか確認中です。 問題なければPR出しますね。

t-tajiri commented 4 years ago

ちなみに、上書きする方法はdate-fnsのadapterを参考にしました。 https://github.com/chartjs/chartjs-adapter-date-fns/blob/master/src/index.js

t-tajiri commented 4 years ago

ざっと1割~2割削減ってところですね。 開発環境で確認しているのか原因はわかっていませんが、若干レンダリングが遅いのが気になります。