Implement i18n's getLocaleByPath function

[matiboux]

Changes

This PR gives a new implementation for the getLocaleByPath function.
It also fixes a parameter name in another getLocaleByPath function, one that is exposed to users.

According to the Internationalization guide, the following behavior is expected:

For a given configuration:

{
    i18n: {
        defaultLocale: "en",
        locales: ["en", "es", "fr", {
            path: "portugues",
            codes: ["pt-AO", "pt", "pt-BR"]
        }]
    }
}

The function is expected to behave as:

import { getLocaleByPath } from "astro:i18n";
console.log(getLocaleByPath("en")) // will log "en"
console.log(getLocaleByPath("fr")) // will log "fr"
console.log(getLocaleByPath("portugues")) // will log "pt-AO"

However, the behavior for the current implementation is:

import { getLocaleByPath } from "astro:i18n";
console.log(getLocaleByPath("en")) // will log "pt-AO"
console.log(getLocaleByPath("fr")) // will log "pt-AO"
console.log(getLocaleByPath("portugues")) // will log "pt-AO"

Explaining the current behavior:

• The current implementation ignores non-object locales (such as "en", "es", "fr").
• The current implementation ignores the given path argument.
• The current implementation returns the first object locale, regardless of the path.

The new implementation I'm proposing:

• Supports non-object locales, assuming locale == path == code.
• Tries to find a match to the path argument.
• Fallbacks to returning undefined if no match was found.

Testing

I don't think this function is tested. I did not add tests.

I just implemented the function in a project of mine, configuring i18n to match the example I gave in the above section, which is the same as the example in the documentation.

Docs

I aimed to match the existing documentation, so no changes are needed.

[changeset-bot]

🦋 Changeset detected

Latest commit: 7a39ff4

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[matiboux]

I painfully generated a changeset, after installing dependencies in a new environment: curl, pnpm, installing packages, git, and finally running the pnpm exec command.

I chose a "minor" bump, because a behavior changed, while still being a fix to match the v4 documentation.

For this new commit, one workflow about the changeset ("Check mergeability / check") failed with HTTP 403 error (see workflow logs). I'll let you investigate why...

[matiboux]

More context for review:

For a working example using my implementation, I finished working on the website where I needed it.
TLDR: Basically, I wanted to make a switch between configured locales and detect the locale for a given URL.

• See the getLocaleByPath function code here: github.com/2gether-asso/app.2gether-asso.fr/blob/main/app/app/src/i18n/getLocaleByPath.ts
• See the code in context, in a PR for adding i18n support: github.com/2gether-asso/app.2gether-asso.fr/pull/6/files

Off-topic:

From my experience, possible suggestions for future general i18n improvement:

• having a method or context to list all configured locales, like getLocales or Astro.locales.
• having a method to get or infer the locale from an URL, like getLocaleByUrl.
• having locale info returned from getAbsoluteLocaleUrlList for list items.

I made clunky functions and used getLocaleByPath to do some of that stuff for my use case, but proper Astro functions could help. Anyway, still context, but getting off-topic. This should probably go into an issue after this, unless no change is possible there.

[matthewp]

From the description this sounds like a bug fix, if so shouldn't this be a minor?

Also, given you have some examples that are not working correctly now, can you add tests?

[matiboux]

From the description this sounds like a bug fix, if so shouldn't this be a minor?

Did you mean "patch"? You're right, I see patches are for "backward compatible bug fixes".
I struggled to get my mind around "backward compatible" as I changed a behavior, but my changes aim to fix the code so it works as described in the documentation so it should qualify as patch.

Also, given you have some examples that are not working correctly now, can you add tests?

Sure, looking into it.

[matiboux]

Following the latest commits:

• Updated the changeset,
• Added the "expected behavior" example I gave in the PR description as a new E2E test.

The CI/CD checks for tests are successful.

Ready for review!

[ematipico]

Thank you for fixing this!