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

Add support for nonExplicitSupportedLngs #1930

Merged
merged 3 commits into from
Aug 21, 2022

Conversation

chrilis
Copy link
Contributor

@chrilis chrilis commented Aug 21, 2022

This PR adds support for nonExplicitSupportedLngs and is related to #1412. This features adds the possibility to support multiple variants of a language (de-AT, de-CH and de-DE) with a single JSON file (e.g. public/locales/de/common.js).

During the work on this PR I noticed that the the default value in fallbackLng should always be loaded. At least this is how I interprete this testcase. Although this is related to #1927, the fix is part of this PR, because nonExplicitSupportedLngs depends on the same logic. Regarding the test-case above: I added en-US to supported locales and removed en from the initialI18nStore, because I think this should be the expected behavior. Is this correct @adrai?

Currently the combination fallbackLng with a function and nonExplicitSupportedLngs: true is not supported. Mainly because I didn't know how this could be implemented 😅 But I guess that the use cases for this combination should be very rare.

I'm looking forward to your feedback @adrai, thank you!

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • run tests npm run test
  • tests are included
  • commit message and code follows the Developer's Certification of Origin

Checklist (for documentation change)

  • only relevant documentation part is changed (make a diff before you submit the PR)
  • motivation/reason is provided
  • commit message and code follows the Developer's Certification of Origin

@chrilis chrilis marked this pull request as draft August 21, 2022 13:43
@chrilis chrilis changed the title Feat/non explicit supported lngs Add support for nonExplicitSupportedLngs Aug 21, 2022
@adrai
Copy link
Member

adrai commented Aug 21, 2022

lgtm

@chrilis chrilis marked this pull request as ready for review August 21, 2022 14:11
@adrai
Copy link
Member

adrai commented Aug 21, 2022

This looks really good to me.
Do you think this could be released with a minor version, or should we go with a major?

@adrai adrai added the enhancement New feature or request label Aug 21, 2022
@chrilis
Copy link
Contributor Author

chrilis commented Aug 21, 2022

I'm not sure. The changes could lead to more languages being loaded, which would increase the page size. I tend to think of that as a breaking change - what do you think?

@adrai adrai merged commit 1b8eb6b into i18next:master Aug 21, 2022
@adrai
Copy link
Member

adrai commented Aug 21, 2022

just released v12.0.0... thank you very much for your great contribution @chrilis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants