dayjs.locale() throws type error due to type update in v.1.8.30

[B1rch]

Describe the bug
The locale() function used to accept a string up until v1.8.30, which updated the types to use LocalePresetType (73813ab). This has broken our CI/CD pipelines, as our i18n implementation returns the locale code as a string.

Expected behavior
Patch versions should not include type changes that are not backwards compatible with previous versions.

Information

• Day.js Version [1.8.30]
• OS: [Ubuntu 16.04]
• Browser [n/a]

suggestion
Add back string as a valid type either in the function declarations or in the LocalePresetType union.

We can work around this issue by casting the string to a LocalePresetType but we would prefer if we didn't have to do this everywhere our i18n interacts with dayjs.

[iamkun]

Sorry for the inconvenience. We will fix this by adding a back string type in LocalePresetType in 1.8.31 shortly.

[iamkun]

One problem is if I added string as a backup type. It will be useless to add these preset strings?

e.g.

type localeName = 'en' | 'es' | string

Typescript will treat and string as valid, and IDE will have no auto-complete at all.

[B1rch]

That is true, and there might be better way to achieve this. But I don't imagine people are hardcoding locale strings all to often. Regardless, not having breaking changes in a patch version has a higher priority over QOL changes if it were up to me.

I haven't looked at the source so I don't know what the previous behavior was, but maybe you can look into adding a warning if the locale code provided to the function is not supported, and perhaps adding the option for a fallback locale to be used in that case.

[iamkun]

Maybe we should roll back to string only instead of the listed locale names.

[muuvmuuv]

Make it a major release and mark it a breaking change would also do it. I prefer stricter typing.

[iamkun]

@muuvmuuv It just comes to me that there might be user-customized locale name, like 'aaa', maybe the only thing we could do is fall back to 'string' type.

[muuvmuuv]

I don't think so because you can extend the type with custom types like this:

type Locale<AdditionalTypes = void> = LocalePresetType | AdditionalTypes

type CustomLocales = 'aaa' | 'bbb'

const one: Locale = 'az'
const two: Locale<CustomLocales> = 'aaa'

I would fall back to string only if dayjs locale gets intelligent enough so that de-DE gets recognized as de, then typings are not longer needed. Or at least append string so the build in locales are typed. Momentjs does it that way like I mentioned in another issue.

[dyoshikawa]

I'm sorry that I suggested this type. #955
It caused the unpredictable inconvenience.

@muuvmuuv 's suggestion looks good to me in the long run.
But in near future, should add string to LocalePresetType as a interim solution, I think.

[iamkun]

This would be a good fix at this moment. #968

[umairhm]

This would be a good fix at this moment. #968

Folks, do we have any timeline for this to be merged and released? Any help would be highly appreciated, thank you!

[iamkun]

🎉 This issue has been resolved in version 1.8.31 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀