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

core bug with locale plugin #2281

Open
titanism opened this issue Apr 11, 2023 · 5 comments
Open

core bug with locale plugin #2281

titanism opened this issue Apr 11, 2023 · 5 comments

Comments

@titanism
Copy link
Contributor

titanism commented Apr 11, 2023

> dayjs('4/23/22', 'M/DD/YY').locale('en').format('LT')
'12:00 AM'
> dayjs('4/23/22', 'M/DD/YY').locale('en').format('LTS')
'12:00:00 AM'
> dayjs('4/23/22', 'M/DD/YY').locale('en').format('L')
'04/23/2022'
> dayjs('4/23/22', 'M/DD/YY').locale('en').format('LL')
'April 23, 2022'
> dayjs('4/23/22', 'M/DD/YY').locale('en').format('LLL')
'April 23, 2022 12:00 AM'
> dayjs('4/23/22', 'M/DD/YY').locale('en').format('LLLL')
'Saturday, April 23, 2022 12:00 AM'
> dayjs('4/23/22', 'M/DD/YY').locale('en').format('l')
Uncaught TypeError: Cannot read properties of undefined (reading 'replace')
    at /Users/user/Projects/web/node_modules/.pnpm/dayjs@1.11.7/node_modules/dayjs/plugin/localizedFormat.js:1:676
    at String.replace (<anonymous>)
    at /Users/user/Projects/web/node_modules/.pnpm/dayjs@1.11.7/node_modules/dayjs/plugin/localizedFormat.js:1:563
    at r.format (/Users/user/Projects/web/node_modules/.pnpm/dayjs@1.11.7/node_modules/dayjs/plugin/localizedFormat.js:1:761)
    at e.format (/Users/user/Projects/web/node_modules/.pnpm/dayjs@1.11.7/node_modules/dayjs/plugin/preParsePostFormat.js:1:592)
    at u.format (/Users/user/Projects/web/node_modules/.pnpm/dayjs@1.11.7/node_modules/dayjs/plugin/utc.js:1:1655)
> dayjs('4/23/22', 'M/DD/YY').locale('zh').format('l')
'2022/4/23'
> dayjs('4/23/22', 'M/DD/YY').locale('de').format('l')
'23.4.2022'

As you can see, there is an uncaught typeerror above for "l", "ll", "lll", and "llll" usage for certain locales, e.g. en.

@titanism
Copy link
Contributor Author

The reason is because the englishFormats object does not contain it.

export const englishFormats = {
LTS: 'h:mm:ss A',
LT: 'h:mm A',
L: 'MM/DD/YYYY',
LL: 'MMMM D, YYYY',
LLL: 'MMMM D, YYYY h:mm A',
LLLL: 'dddd, MMMM D, YYYY h:mm A'
}

Needs to have l, ll, lll, and llll added. Will submit a PR now.

@titanism
Copy link
Contributor Author

This has been tested. Confirmed my PR works per #2282.

Please merge and release to npm with patch version bump.

Ping me back once done please! Thank you @iamkun.

> dayjs('4/23/22', 'M/DD/YY').locale('en').format('l') // this is with the PR
'4/23/2022'

titanism added a commit to forwardemail/forwardemail.net that referenced this issue Apr 11, 2023
@tendguo
Copy link

tendguo commented Apr 14, 2023

I don't agree with your conclusion, because I looked up the source code and found this

const B = b && b.toUpperCase()
return a || formats[b] || englishFormats[b] || t(formats[B])

@titanism
Copy link
Contributor Author

@tendguo you are incorrect as that output is undefined, and you get an uncaught exception TypeError. From my tests on the latest version of dayjs it is an error when you require all the locales and then run the above commands.

@iamkun
Copy link
Owner

iamkun commented Apr 16, 2023

there should be no problem with localizedFormat plugin

https://runkit.com/embed/yzvddg5czfg6

titanism added a commit to forwardemail/forwardemail.net that referenced this issue May 21, 2023
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

No branches or pull requests

3 participants