Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(date): Dateの章のリファクタリング #1515

Merged
merged 3 commits into from
Nov 5, 2022
Merged

Conversation

azu
Copy link
Collaborator

@azu azu commented Nov 5, 2022

No description provided.

@bot-user
Copy link

bot-user commented Nov 5, 2022

Deploy Preview for js-primer ready!

Name Link
🔨 Latest commit effd95c
🔍 Latest deploy log https://app.netlify.com/sites/js-primer/deploys/6365bd856e37cc000eab6fde
😎 Deploy Preview https://deploy-preview-1515--js-primer.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Comment on lines -99 to +102
コンストラクタ関数に2つ以上の引数を渡すと、このオーバーロードが適用されます。
日を表す第三引数から後ろの引数は省略可能ですが、日付だけはデフォルトで1が設定され、そのほかには0が設定されます。
また、月を表す第二引数は0から11までの数値で指定することにも注意しましょう。
コンストラクタ関数に2つ以上の引数を渡すと、この方法で`Date`インスタンスが作成されます。
月内の日を表す第三引数(`day`)から後ろの引数は省略可能です。
省略した場合のそれぞれの引数の初期値は`0`ですが、日(`day`)を表す第三引数だけは`1`がデフォルト値となります。
また、月(`month`)を表す第二引数は`0`が1月に対応し、`0`から`11`までの数値で月を指定することにも注意しましょう。
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

全体的に引数を明示するようにした。

日付だけはデフォルトで1が設定され

というのがよくわからなかったけど、多分 day のことかな? @lacolaco

Copy link
Collaborator

Choose a reason for hiding this comment

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

多分そうですね


そのため、JavaScriptにおける日付・時刻の処理は、標準のDateではなくライブラリを使うことが一般的になっています。
代表的なライブラリとしては、[moment.js][]や[js-joda][]、[date-fns][]などがあります。
代表的なライブラリとしては、[Day.js][]、[date-fns][]、[js-joda][]、[moment.js][]の後継である[Luxon][]などがあります。
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

moment.jsはdeprecatedなのでLuxonを追加。
Day.jsも一応入れた

Comment on lines -190 to +196
// moment.jsで現在時刻のmomentオブジェクトを作る
const now = moment();
// Day.jsで現在時刻のDay.jsオブジェクトを作る
const now = dayjs();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ここもDay.jsにした。メソッドが一番momentに感覚が近いので。

```

<!-- ECMA-402 と Temporalについてを書く -->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

これどうしようかすごい迷う。
[ECMA-402][] で国際化が進められているのと、temporalはすごい微妙な時期。

ライブラリにずっと依存してるんじゃなくて、仕様としてもDateを直そうと進んでいるよって雰囲気を書きたいけど、Proposal段階だからどうするかなーという感じ。
名前を出すべきかどうか。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#1495 (comment)
次のミーティングアジェンダに入れた

// formatメソッドで任意の書式の文字列に変換する
console.log(future.format("YYYY/MM/DD"));
console.log(future.format("YYYY/MM/DD HH:mm"));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

これ 10分進めてるのに mm がなかったので

Comment on lines -59 to +60
この方法は実行環境による挙動の違いが起きないので安全です
また、時刻値を直接指定するので、他の2つの方法と違ってタイムゾーンを考慮する必要がありません。
この方法は基準となる時刻とタイムゾーンが固定されているため、実行環境のタイムゾーンによる違いが起きないので安全です
そのため、他の2つの方法と違ってタイムゾーンを考慮する必要がありません。
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

同じことを言ってる感じがしたので、ちょっとタイムゾーンと明確にした。
実行環境の時計でずれるとかはあるため。

@azu azu merged commit a10e0cd into master Nov 5, 2022
@azu azu deleted the review-date branch November 5, 2022 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants