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

Fallback to language only locale + support uppercase locales #1524

Merged

Conversation

codeability-ab
Copy link
Contributor

Fallback to locale xx if xx-yy is provided and does not exist

Also support locales in uppercase such as en-GB

@codecov
Copy link

codecov bot commented Jun 6, 2021

Codecov Report

Merging #1524 (816f1e8) into dev (c5688f3) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               dev     #1524   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          176       176           
  Lines         1980      1984    +4     
  Branches       502       503    +1     
=========================================
+ Hits          1980      1984    +4     
Impacted Files Coverage Δ
src/index.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5688f3...816f1e8. Read the comment docs.

@codeability-ab
Copy link
Contributor Author

@iamkun will this pull request be considered for merge?

src/index.js Outdated
Ls[presetLower] = object
l = presetLower
}
if (!l && preset.split('-').length > 1) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can simplify the logic to
const presetSplit = preset.split('-')
if (!l && presetSplit.length) {
return parseLocale(presetSplit[0])
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I'll fix it!

@@ -96,6 +96,16 @@ it('Uses the localized lowercase formats if defined', () => {
['l', 'll', 'lll', 'llll'].forEach(option => expect(znDate.format(option)).toBe(znDate.format(znCn.formats[option])))
})

it('Uses fallback to xx if xx-yy not available', () => {
expect(dayjs('2019-02-01').locale('es-yy').format('L'))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will be more clear to use format('MMMM') to test if the locale is correct and not the default English

l = presetLower
}
const presetSplit = preset.split('-')
if (!l && presetSplit.length > 1) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (!l && presetSplit.length) {

we could just check the length is above 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my first attempt but other unit tests break if I don't check that the split actually splits the preset string (i.e it results in more than one array element).

The test "Not supported locale string fallback to previous one (instance)" in locale.test.js fails with "if (!l && presetSplit.length) {"

The "not_supported_locale_string" is split into one element => recursive call, "not_supported_locale_string" is split into one element again => recursive call ... etc

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh. I see, you can just change the "not_supported_locale_string" test string 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean like "not-supported-locale-string"?
But that string will never split into something valid.
I still think it's a good idea to only recurse if the preset actually splits into 2 parts.

@codeability-ab
Copy link
Contributor Author

@iamkun is there anything stopping you from merging? Anything I need to do?

@hirbod
Copy link

hirbod commented Feb 11, 2022

I would love to see this PR merged. It is exactly fixing all the issues I've encountered on my React Native App, when trying to load the correct locale based on system preferences

@codeability-ab
Copy link
Contributor Author

@iamkun Can we merge this one please?

@iamkun iamkun merged commit 9138dc2 into iamkun:dev Feb 28, 2022
@iamkun
Copy link
Owner

iamkun commented Feb 28, 2022

THX

iamkun pushed a commit that referenced this pull request Mar 14, 2022
# [1.11.0](v1.10.8...v1.11.0) (2022-03-14)

### Bug Fixes

* Add Kirundi (rn) locale ([#1793](#1793)) ([74e5247](74e5247))
* add missing date shorthand D type definition ([#1752](#1752)) ([b045baf](b045baf))
* Add relative time to Galician (gl) and fix ordinals ([#1800](#1800)) ([dcbf170](dcbf170))
* update German locales (de-at, de-ch) ([#1775](#1775)) ([f9055a7](f9055a7))
* update Icelandic [is] locale relativeTime config ([#1796](#1796)) ([76f9e17](76f9e17))
* Update index.d.ts note ([#1716](#1716)) ([5a108ff](5a108ff))
* Update locale German [de] monthsShort ([#1746](#1746)) ([4a7b7d0](4a7b7d0))
* update meridiem function to Kurdish (ku) locale ([#1725](#1725)) ([efd3904](efd3904))
* update updateLocal plugin typescript types ([#1692](#1692)) ([c7a3f73](c7a3f73))

### Features

* Fallback to language only locale + support uppercase locales ([#1524](#1524)) ([9138dc2](9138dc2))
@iamkun
Copy link
Owner

iamkun commented Mar 14, 2022

🎉 This PR is included in version 1.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@codeability-ab codeability-ab deleted the issue-1523/fallback-to-language-only-locale branch March 20, 2022 18:29
andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
# [1.11.0](iamkun/dayjs@v1.10.8...v1.11.0) (2022-03-14)

### Bug Fixes

* Add Kirundi (rn) locale ([#1793](iamkun/dayjs#1793)) ([74e5247](iamkun/dayjs@74e5247))
* add missing date shorthand D type definition ([#1752](iamkun/dayjs#1752)) ([b045baf](iamkun/dayjs@b045baf))
* Add relative time to Galician (gl) and fix ordinals ([#1800](iamkun/dayjs#1800)) ([dcbf170](iamkun/dayjs@dcbf170))
* update German locales (de-at, de-ch) ([#1775](iamkun/dayjs#1775)) ([f9055a7](iamkun/dayjs@f9055a7))
* update Icelandic [is] locale relativeTime config ([#1796](iamkun/dayjs#1796)) ([76f9e17](iamkun/dayjs@76f9e17))
* Update index.d.ts note ([#1716](iamkun/dayjs#1716)) ([5a108ff](iamkun/dayjs@5a108ff))
* Update locale German [de] monthsShort ([#1746](iamkun/dayjs#1746)) ([4a7b7d0](iamkun/dayjs@4a7b7d0))
* update meridiem function to Kurdish (ku) locale ([#1725](iamkun/dayjs#1725)) ([efd3904](iamkun/dayjs@efd3904))
* update updateLocal plugin typescript types ([#1692](iamkun/dayjs#1692)) ([c7a3f73](iamkun/dayjs@c7a3f73))

### Features

* Fallback to language only locale + support uppercase locales ([#1524](iamkun/dayjs#1524)) ([9138dc2](iamkun/dayjs@9138dc2))
andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
# [1.11.0](iamkun/dayjs@v1.10.8...v1.11.0) (2022-03-14)

### Bug Fixes

* Add Kirundi (rn) locale ([#1793](iamkun/dayjs#1793)) ([74e5247](iamkun/dayjs@74e5247))
* add missing date shorthand D type definition ([#1752](iamkun/dayjs#1752)) ([b045baf](iamkun/dayjs@b045baf))
* Add relative time to Galician (gl) and fix ordinals ([#1800](iamkun/dayjs#1800)) ([dcbf170](iamkun/dayjs@dcbf170))
* update German locales (de-at, de-ch) ([#1775](iamkun/dayjs#1775)) ([f9055a7](iamkun/dayjs@f9055a7))
* update Icelandic [is] locale relativeTime config ([#1796](iamkun/dayjs#1796)) ([76f9e17](iamkun/dayjs@76f9e17))
* Update index.d.ts note ([#1716](iamkun/dayjs#1716)) ([5a108ff](iamkun/dayjs@5a108ff))
* Update locale German [de] monthsShort ([#1746](iamkun/dayjs#1746)) ([4a7b7d0](iamkun/dayjs@4a7b7d0))
* update meridiem function to Kurdish (ku) locale ([#1725](iamkun/dayjs#1725)) ([efd3904](iamkun/dayjs@efd3904))
* update updateLocal plugin typescript types ([#1692](iamkun/dayjs#1692)) ([c7a3f73](iamkun/dayjs@c7a3f73))

### Features

* Fallback to language only locale + support uppercase locales ([#1524](iamkun/dayjs#1524)) ([9138dc2](iamkun/dayjs@9138dc2))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants