Skip to content
This repository was archived by the owner on Sep 27, 2023. It is now read-only.

Fix DateParam.parser#74

Merged
shoe116 merged 1 commit intodevelopmentfrom
fix-date-parser
Jan 7, 2022
Merged

Fix DateParam.parser#74
shoe116 merged 1 commit intodevelopmentfrom
fix-date-parser

Conversation

@akiomik
Copy link
Copy Markdown
Contributor

@akiomik akiomik commented Jan 7, 2022

以下のバグを修正します。

  • BCODE(ticker, day, prop) を実行したときの day が意図しない日付に解釈されてしまうバグを修正

@akiomik akiomik added the bug Something isn't working label Jan 7, 2022
@akiomik akiomik requested a review from shoe116 January 7, 2022 10:22
@akiomik akiomik self-assigned this Jan 7, 2022
const month = parseInt(matches[2], 10)
const day = parseInt(matches[3], 10)
return new DateParam(new Date(year, month, day))
return new DateParam(new Date(str))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

バグ修正の本体です。2つのバグがありました。

  • new Date() の引数は new Date(year, month, day) ではなく new Date(year, monthIndex, day) なので、 指定月-1 を渡すのが正しいのだが、そのまま月を渡していた
  • new Date(year, monthIndex, day) 形式で Date を生成すると自動的にタイムゾーンが GMT+0900 (Japan Standard Time) として解釈されるものの、 toISOString() 時に T15:00:00.000Z とUTCに自動的に変換されてしまうために、ロケールが日本の場合には-1日されて解釈されてしまう (?)

上記の問題はどちらも new Date('YYYY-MM-DD') のように文字列型で渡すことで解決したので、そのようにしています。

参考: https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/Date/Date

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

文字列渡すほうがむしろ安全っていうのがちょい残念だけど、なるほど。
真ん中が monthIndex だったの、見るたびに「あーそうだった」ってなるな・・・

@shoe116 shoe116 merged commit a58fdeb into development Jan 7, 2022
@shoe116 shoe116 deleted the fix-date-parser branch January 7, 2022 10:38
@akiomik akiomik mentioned this pull request Jan 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants