iamkun / dayjs

⏰ Day.js 2kB immutable date-time library alternative to Moment.js with the same modern API
https://day.js.org
MIT License
46.72k stars 2.28k forks source link

Incorrect year with localized month in date #2511

Open Vansinnesvisor opened 10 months ago

Vansinnesvisor commented 10 months ago

Describe the bug When use customParseFormat plugin and localized string with latest month of year e.g. "1 декабря 2023" I'm getting the wrong year (+1).

Base code

import dayjs from "dayjs"
import customParseFormat from "dayjs/plugin/customParseFormat"
import "dayjs/locale/ru"

dayjs.extend(customParseFormat)
dayjs.locale("ru")

Example

//Check various months
console.log("JAN:", dayjs("1 января 2023", "D MMMM YYYY").format("DD.MM.YYYY")); //JAN: 01.01.2023
console.log("NOV:", dayjs("1 ноября 2023", "D MMMM YYYY").format("DD.MM.YYYY")); //NOV: 01.11.2023
console.log("DEC:", dayjs("1 декабря 2023", "D MMMM YYYY").format("DD.MM.YYYY")); //DEC: 01.12.2024
// Check end of year
console.log("END:", dayjs().endOf("year").format("DD.MM.YYYY")); //END: 31.12.2023

Expected behavior

console.log("DEC:", dayjs("1 декабря 2023", "D MMMM YYYY").format("DD.MM.YYYY")); //Must be: 01.12.2023

Information

kavish5 commented 9 months ago

I've debug the issue related to date parsing mentioned above, and found out it exist at least in Russian and Ukrainian. Please note that I am not a language expert; my observations are based on the behavior I encountered in the code.

Observed Behavior The issue occurs when parsing dates in Russian and Ukrainian languages. Here are the console logs demonstrating the problem:

console.log("DEC:", dayjs("1 декабря 2023", "D MMMM YYYY").format("DD.MM.YYYY")); console.log("DEC:", dayjs("1 грудня 2023", "D MMMM YYYY").format("DD.MM.YYYY")

Temporary Workaround A quick fix, for those needing an immediate solution without a formal fix from the dayjs library, is to adjust the function call as follows:

console.log("DEC:", dayjs("1 декабрь 2023", "D MMMM YYYY г."));

My understanding is that both "декабрь" and "декабря" are variants of December in Russian.

Potential Code-Level Issue Upon investigating the dayjs library's code, specifically in the /src/plugin/customParseFormat/index.js file, I believe I've located the potential source of the issue:

The parseFormattedInput function internally calls const parser = makeParser(format). The issue seems to be rooted in how the parser processes the month format. Here’s the relevant snippet:

MMMM: [ matchWord, function(input) { const months = getLocalePart('months'); const matchIndex = months.indexOf(input) + 1; if (matchIndex < 1) { throw new Error(); } this.month = matchIndex % 12 || matchIndex; } ]

The getLocalePart function retrieves a list of 24 items, representing stand-alone months and formatted months. The parser's handling of December (декабря in the example) returns an index of 24, which is incorrectly interpreted as the month number. As a result, the parser output becomes { day: 1, month: 24, year: 2023 }, inadvertently shifting the year to 2024.

Request for Confirmation and Priority Assignment I request the repository maintainers to confirm this as a bug and determine its priority for fixing. My findings suggest a potential solution would involve adjusting the month parsing logic to correctly handle language-specific month variations.

Vansinnesvisor commented 9 months ago

@kavish5 thank you for your detailed answer. The word "декабрь" in Russian directly denotes the name of the month - December. In combination with the day of the month we use the word "декабря". So the string "1 декабря" is correct, but "1 декабрь" is not. Therefore, we came up with a simple but rough solution to this issue until an official fix comes out.

let raw_date = "1 декабря 2023";
raw_date = raw_date.replace("декабря", "декабрь");
let correct_date = dayjs(raw_date, "D MMMM YYYY").format("DD.MM.YYYY"); //return 01.12.2023

We only replace the last month because other months are always converted correctly.

kavish5 commented 9 months ago

Just for the curiosity, was the replace solution already implemented at your end before this @Vansinnesvisor?

I do have check the code and it looks fixable but not sure should I create a PR without @iamkun confirmation. This issue has been raised before for Polish language as well.

Vansinnesvisor commented 9 months ago

@kavish5 this temporary solution has been implemented in our project immediately after we discovered a problem with incorrect date conversion. I checked the issue with Polish language and I think is relevant for most slavic languages.

ahujamoh commented 9 months ago

I agree the issue #2483 is quite similar to this one

artischockee commented 9 months ago

The corresponding Pull Request is waiting for merge since March 3, 2023 (https://github.com/iamkun/dayjs/pull/2251).

Hope the maintainer will react to this ASAP. It would probably be cool if everyone interested in this fix approves this PR.