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

feat: added support for showing the default locale in the url #51

Conversation

Alexandre-Fernandez
Copy link
Contributor

  • added a property to the astro-i18next config showDefaultLocale, to set the behaviour of the routing concerning the default locale (localizePath & generate)
  • added a better way to use the astro-i18next config in the runtime (made the necessary modifications for the config's routes property to use this new way)

@yassinedoghri
Copy link
Owner

Hey, thanks for the PR!

Haven't checked it in details though, hard to find the time nowadays, I'll try reviewing it later. Though, if I understand correctly, this allows the default language to behave like other languages. So, for example if the default language is en, then it'll be set in both https://example.com/ and https://example.com/en, right? And also, why such a feature?

Having the astro-i18next config available at runtime is a nice touch 👍

@Alexandre-Fernandez
Copy link
Contributor Author

Alexandre-Fernandez commented Oct 11, 2022

So, for example if the default language is en, then it'll be set in both https://example.com/ and https://example.com/en

Yes, it makes the default locale behave like a normal one if the setting is turned on.

why such a feature?

  • This is a very common pattern for many websites
  • SEO
  • UX (user can directly replace the language code in the URL without getting a 404)
  • No favoritism (some countries have two or more languages, having a special treatment for one of them may not be wanted by the owners).
  • Consistency

Next.js i18n had a discussion about this if you're interested : vercel/next.js#18419

@yassinedoghri
Copy link
Owner

Thank you again for the PR, it's merged.

I'll have to clean things up in the develop branch and add a few notes in the docs before releasing. Currently busy on another project, hopefully I'll have time by the end of next week 🤞

@@ -0,0 +1,16 @@
import { AstroI18nextConfig } from "types";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be "./types". This is breaking the project for me right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry for that one the auto-import messed up :/

Copy link
Contributor

Choose a reason for hiding this comment

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

I submitted a couple PRs to resolve this

yassinedoghri pushed a commit that referenced this pull request Nov 6, 2022
- default locale can now behave as other locales and be displayed in the url
- initialize astro-i18next config and to make it available at runtime

refs #54
yassinedoghri pushed a commit that referenced this pull request Nov 6, 2022
# [1.0.0-beta.13](v1.0.0-beta.12...v1.0.0-beta.13) (2022-11-06)

### Bug Fixes

* add isFileHidden function + tests to prevent missing hidden files ([7dcd0aa](7dcd0aa))
* **generate:** replace isLocale check with user defined locales to prevent nested folders generation ([a598e2e](a598e2e)), closes [#56](#56)
* **i18next-server:** load locale files synchronously ([e7892e2](e7892e2))
* update types import to relative ([#58](#58)) ([44a5422](44a5422))

### Features

* add option to show the default locale in the url ([#51](#51)) ([ea939db](ea939db)), closes [#54](#54)
* add support for route translations ([db5200b](db5200b)), closes [#50](#50) [#29](#29)
* allow implicit key for <Trans> when omitting i18nKey prop ([ff14354](ff14354)), closes [#53](#53)
* simplified API + instanciate i18next both in server and client side ([ed44510](ed44510)), closes [#57](#57) [#46](#46) [#37](#37)

### BREAKING CHANGES

* - defaultLanguage is now defaultLocale
- supportedLanguages is now locales
- i18next config is now split into two configs: `i18nextServer`
and `i18nextClient`
AliLee0923 pushed a commit to AliLee0923/astro-i18N that referenced this pull request Dec 2, 2023
# [1.0.0-beta.13](yassinedoghri/astro-i18next@v1.0.0-beta.12...v1.0.0-beta.13) (2022-11-06)

### Bug Fixes

* add isFileHidden function + tests to prevent missing hidden files ([7dcd0aa](yassinedoghri/astro-i18next@7dcd0aa))
* **generate:** replace isLocale check with user defined locales to prevent nested folders generation ([a598e2e](yassinedoghri/astro-i18next@a598e2e)), closes [#56](yassinedoghri/astro-i18next#56)
* **i18next-server:** load locale files synchronously ([e7892e2](yassinedoghri/astro-i18next@e7892e2))
* update types import to relative ([#58](yassinedoghri/astro-i18next#58)) ([44a5422](yassinedoghri/astro-i18next@44a5422))

### Features

* add option to show the default locale in the url ([#51](yassinedoghri/astro-i18next#51)) ([ea939db](yassinedoghri/astro-i18next@ea939db)), closes [#54](yassinedoghri/astro-i18next#54)
* add support for route translations ([db5200b](yassinedoghri/astro-i18next@db5200b)), closes [#50](yassinedoghri/astro-i18next#50) [#29](yassinedoghri/astro-i18next#29)
* allow implicit key for <Trans> when omitting i18nKey prop ([ff14354](yassinedoghri/astro-i18next@ff14354)), closes [#53](yassinedoghri/astro-i18next#53)
* simplified API + instanciate i18next both in server and client side ([ed44510](yassinedoghri/astro-i18next@ed44510)), closes [#57](yassinedoghri/astro-i18next#57) [#46](yassinedoghri/astro-i18next#46) [#37](yassinedoghri/astro-i18next#37)

### BREAKING CHANGES

* - defaultLanguage is now defaultLocale
- supportedLanguages is now locales
- i18next config is now split into two configs: `i18nextServer`
and `i18nextClient`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants