Fix absolute-date #32375
Fix absolute-date #32375
Conversation
GiteaBot
added
the
lgtm/need 2
pull-request-size
bot
added
the
size/M
GiteaBot
added
lgtm/need 1
GiteaBot
added
lgtm/done
wolfogre
added
the
reviewed/wait-merge
GiteaBot
removed
the
reviewed/wait-merge
| @@ -7,9 +7,15 @@ test('toAbsoluteLocaleDate', () => { | |||
| day: 'numeric', | |||
| })).toEqual('March 15, 2024'); | |||
|
|
|||
| expect(toAbsoluteLocaleDate('2024-03-15', 'de-DE', { | |||
| expect(toAbsoluteLocaleDate('2024-03-15T01:02:03', 'de-DE', { | |||
| year: 'numeric', | |||
| month: 'long', | |||
| day: 'numeric', | |||
| })).toEqual('15. März 2024'); | |||
There was a problem hiding this comment.
What is the output of the function for inputs 2024-03-15, de-DE now?
There are cases where only date without time is passed to toAbsoluteLocaleDate, for example for milestones. I think this change may break those cases depending on the timezone in which it is ran.
I think Temporal.PlainDate is the only JS API capable of dealing correctly with date-only values (Date can not as it deals with timestamps that will shift with time zone).
There was a problem hiding this comment.
The problem is:
> new Date("2024-03-15")
Date Fri Mar 15 2024 01:00:00 GMT+0100 (Central European Standard Time)This date object has a time component which is wrong because dates are timeless. Is it guaranteed that the output of the following is always the same, regardless of system time and timezone?
> new Date("2024-03-15").toLocaleString("de-DE", {year: "numeric", month: "long", day: "numeric"})
15. März 2024There was a problem hiding this comment.
It is not related.
Using a backend date string is wrong, for long time. That string is in server's timezone, is a string, doesn't have the timezone field, and frontend don't know, it would definitely go wrong when the browser's timezone is different from the server's.
To make the "deadline" right, it needs to completely refactor the backend first.
ps: the frontend use the string "date" part is also wrong, but I didn't change that logic.
There was a problem hiding this comment.
Then we only need to do new Date(date + 'T00:00:00'), then the result is right.
There was a problem hiding this comment.
Even easier reproduction, this should be possible to simulate in unit tests:
> process.env.TZ = "US/Eastern"; new Date("2024-03-15").toLocaleString("de-DE", {year: "numeric", month: "long", day: "numeric"})
'14. März 2024'
> process.env.TZ = "Europe/Berlin"; new Date("2024-03-15").toLocaleString("de-DE", {year: "numeric", month: "long", day: "numeric"})
'15. März 2024'There was a problem hiding this comment.
Then we only need to do
new Date(date + 'T00:00:00'), then the result is right.
Yes, but why abuse an API that was not made for this use case? Temporal.PlainDate was made for this.
There was a problem hiding this comment.
-> Fix toAbsoluteLocaleDate and add more tests #32387
Added more tests.
Temporal.PlainDatewas made for this.
It just introduces more unnecessary polyfills, make the code unnecessary larger, and according to MDN: It is not yet ready for production use! (just quoted the sentence).
There was a problem hiding this comment.
It is not yet ready for production use
Right, it's still in stage 3, so not finalized and will probably take another year or two until we see it being implemented in browsers.
* origin/main: (21 commits) Fix toAbsoluteLocaleDate and add more tests (go-gitea#32387) Respect UI.ExploreDefaultSort setting again (go-gitea#32357) Fix absolute-date (go-gitea#32375) Fix undefined errors on Activity page (go-gitea#32378) Add new [lfs_client].BATCH_SIZE and [server].LFS_MAX_BATCH_SIZE config settings. (go-gitea#32307) remove unused call to $.HeadRepo in view_title template (go-gitea#32317) Fix clean tmp dir (go-gitea#32360) Optimize branch protection rule loading (go-gitea#32280) Suggestions for issues (go-gitea#32327) Migrate vue components to setup (go-gitea#32329) Fix db engine (go-gitea#32351) Refactor the DB migration system slightly (go-gitea#32344) Fix broken image when editing comment with non-image attachments (go-gitea#32319) Fix disable 2fa bug (go-gitea#32320) Upgrade rollup to 4.24.0 (go-gitea#32312) Upgrade vue to 3.5.12 (go-gitea#32311) Make admins adhere to branch protection rules (go-gitea#32248) Prevent from submitting issue/comment on uploading (go-gitea#32263) Add warn log when deleting inactive users (go-gitea#32318) Add `DISABLE_ORGANIZATIONS_PAGE` and `DISABLE_CODE_PAGE` settings for explore pages and fix an issue related to user search (go-gitea#32288) ...
* giteaofficial/main: Fix suggestions for issues (go-gitea#32380) refactor: remove redundant err declarations (go-gitea#32381) Fix the missing menu in organization project view page (go-gitea#32313) Fix toAbsoluteLocaleDate and add more tests (go-gitea#32387) Respect UI.ExploreDefaultSort setting again (go-gitea#32357) Fix absolute-date (go-gitea#32375) Fix undefined errors on Activity page (go-gitea#32378) Add new [lfs_client].BATCH_SIZE and [server].LFS_MAX_BATCH_SIZE config settings. (go-gitea#32307) remove unused call to $.HeadRepo in view_title template (go-gitea#32317) Fix clean tmp dir (go-gitea#32360) Optimize branch protection rule loading (go-gitea#32280) Suggestions for issues (go-gitea#32327) Migrate vue components to setup (go-gitea#32329)
No description provided.