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.86k stars 2.29k forks source link

Parsing localized time string including meridiem with format 'a hh:mm' ignores meridiem. #2031

Open rodumani opened 2 years ago

rodumani commented 2 years ago

Describe the bug

Parsing localized time string including meridiem with format 'a hh:mm' ignores meridiem.

In Korean, "am" is "오전" and "pm" is "오후". There is no uppercase/lowercase in Korean.

However, dayjs customParseFormat doesn't recognize the meridiem and only returns hours in am.

Expected behavior A clear and concise description of what you expected to happen.

import dayjs from 'dayjs';
import 'dayjs/locale/ko.js';

import customParseFormat from 'dayjs/plugin/customParseFormat.js';
dayjs.extend(customParseFormat);

const amStr = '오전 10:00';
const pmStr = '오후 10:00';
const format = 'a h:mm';

const am = dayjs(amStr, format);
console.assert(am.hour() === 10, 'am.hour() === 10');
const pm = dayjs(pmStr, format);
console.assert(pm.hour() === 22, 'pm.hour() === 22');
$ node index.js
Assertion failed: pm.hour() === 22

The second assert should be passed, but it fails because it returns the hour as 10.

Information

BePo65 commented 2 years ago

The problem stems from the fact that CustomPareFormat tries to implement some kind of a 'meridiemParse' function that uses the 'meridiem' function of the locale file to find out which hour values are considered 'am' and 'pm'. This function supposes that the 'meridiem' function returns the string for 'pm' for anything after '12' and the string for 'am' for hours '12' and before.

But some locales consider noon as 'pm' among them 'ko'.

If we look to the details of meridiem then we will find that noon / midnight is a very problematic case (and that the chinese locales use more verbs for time ranges, not only 'am' and 'pm'). Details can be found e.g. in wikipdia.

The question is, if we need some kind of 'meridiemParse' function like moment does. @iamkun any opinion on this? I think this would not be too problematic.