-
Notifications
You must be signed in to change notification settings - Fork 663
feat(datetime): add month and day of week constants #6910
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6910 +/- ##
=======================================
Coverage 94.15% 94.16%
=======================================
Files 583 583
Lines 42792 42811 +19
Branches 6815 6815
=======================================
+ Hits 40292 40314 +22
+ Misses 2448 2446 -2
+ Partials 52 51 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Interesting proposal. new Temporal.PlainDate(2025, MAR, 1); // incorrect month |
|
Thank you for pointing that out — I hadn't fully considered the Temporal API implications. That said, I'm curious about the timeline: even after Temporal stabilizes, when would To avoid future confusion, what do you think about providing these constants under a more specific path, such as -import { MAR } from "@std/datetime/constants";
+import { MAR } from "@std/datetime/date/constants";
new Date(2025, MAR, 1) // unambiguously MarchI fully agree with removing these constants once they are no longer needed. |
datetime/constants.ts
Outdated
| * ```ts | ||
| * import { MAR } from "@std/datetime/constants"; | ||
| * | ||
| * MAR; // 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make the example more practical one like new Date(2025, MAR, 1);?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the examples to show practical usage: a8e5658
43a42ca to
a8e5658
Compare
|
I don't think I checked major date libraries such as moment.js, date-fns, day.js, and found that they all still use 0-based month index. I guess maybe these constants still have some utilities. Anyways |
kt3k
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
I think the const names should not be abbreviated. |
|
I understand the preference for non-abbreviated names, but I think:
|
It is more about consistency in std. |
|
Addressed in #6939 |
Summary
Add month index constants (JAN-DEC) and day of week constants (SUN-SAT) to
@std/datetime/constants.Motivation
In many languages and cultures (such as Japanese, Chinese, and Korean), months are referred to by their ordinal numbers: January is "1月" (month 1), February is "2月" (month 2), and so on.
However, JavaScript's
Dateobject uses 0-based indices for months (0 = January, 11 = December). This mismatch between human intuition and the API frequently leads to off-by-one bugs:By providing named constants, developers can write clearer, less error-prone code:
Changes