Conversation
The fallback UI locale (used when neither the visitor's cookie nor Accept-Language matches a shipped catalogue) was only settable at build time via NEXT_PUBLIC_DEFAULT_LOCALE, so changing it meant a custom image. Add a DEFAULT_LOCALE config key (env var or admin dashboard, Settings → Default Language) resolved at request time: - lib/admin/default-locale.ts resolves admin/env > build default and is used by the next-intl request config, the proxy (one intl middleware per default locale seen, since next-intl bakes it in) and /api/config. - The client mirrors the resolved value (i18n/runtime-default-locale.ts) so 'as-needed' URL prefixing in deep links agrees with the server. - Locale autonyms move to i18n/locale-names.ts, shared by the language switcher and the new admin select. Regional tags still map onto catalogues at match time (pt-BR → pt), so DEFAULT_LOCALE=pt is the setting for a Brazilian deployment.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical and moderate findings remain in locale resolution, client propagation, deep-link handling, and admin selection.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
Adds runtime-configurable fallback locales via environment or admin settings, while preserving build-time behavior for Lite builds.
Changes:
- Resolves fallback locales server-side and in middleware.
- Synchronizes API, client, and deep-link behavior.
- Adds admin UI, locale metadata, tests, and documentation.
| File | Description |
|---|---|
README.md |
Documents runtime locale configuration. |
proxy.ts |
Caches locale-specific middleware instances. |
lib/deep-links.ts |
Uses the runtime fallback locale. |
lib/admin/types.ts |
Adds DEFAULT_LOCALE configuration. |
lib/admin/default-locale.ts |
Resolves configured fallback locales. |
lib/admin/__tests__/default-locale.test.ts |
Tests server locale resolution. |
i18n/runtime-default-locale.ts |
Provides the client-side locale mirror. |
i18n/request.ts |
Applies runtime fallback during requests. |
i18n/locale-names.ts |
Centralizes locale display names. |
i18n/__tests__/runtime-default-locale.test.ts |
Tests client locale handling. |
hooks/use-config.ts |
Loads the runtime locale into the client. |
FEATURES.md |
Updates locale configuration documentation. |
components/ui/language-switcher.tsx |
Uses shared locale names. |
app/api/config/route.ts |
Exposes the effective default locale. |
app/(main)/admin/_tabs/settings.tsx |
Adds the Default Language setting. |
.env.example |
Documents locale environment variables. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * been loaded (every request path calls ensureLoaded() first). | ||
| */ | ||
| export function resolveDefaultLocale(): Locale { | ||
| const configured = configManager.get<string>('defaultLocale', '').trim(); |
| if (routing.localePrefix === 'always') { | ||
| withLocale = `/${resolved}${normalized === '/' ? '' : normalized}`; | ||
| } else if (routing.localePrefix === 'as-needed' && resolved !== routing.defaultLocale) { | ||
| } else if (routing.localePrefix === 'as-needed' && resolved !== getDefaultLocale()) { |
Comment on lines
+451
to
+453
| # DEFAULT_LOCALE is read at runtime (also settable in the admin dashboard); | ||
| # NEXT_PUBLIC_DEFAULT_LOCALE is the build-time equivalent and is what the | ||
| # static Lite build uses. |
Review follow-ups: - resolveDefaultLocale reads the admin value as unknown and only trims a string, so a malformed config.json (null, number) is ignored instead of throwing in the proxy on every request. - getLocaleFromPath falls back to the deployment default locale rather than a hard-coded 'en'. With 'as-needed' prefixing and a runtime default of e.g. pt, links generated from an unprefixed URL like /mail stayed unprefixed instead of becoming /en/... . - .env.example: say up front which i18n variables are runtime (DEFAULT_LOCALE) and which are build-time (NEXT_PUBLIC_*).
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Makes the fallback UI locale (used when neither the visitor's
NEXT_LOCALEcookie norAccept-Languagematches a shipped catalogue) configurable at runtime:DEFAULT_LOCALEenv var or Admin → Settings → Default Language.Today this is only
NEXT_PUBLIC_DEFAULT_LOCALE, baked in at build time, so anyone on the published Docker image who wants e.g. Portuguese as the fallback has to build their own image. Per-visitor precedence is unchanged: the user's own choice, then browser language, then this fallback.How
lib/admin/types.ts–CONFIG_ENV_MAP.defaultLocale(DEFAULT_LOCALE, enum over the shipped locales,''= build default). Being an enum,PATCH /api/admin/configalready rejects anything else.lib/admin/default-locale.ts–resolveDefaultLocale(): admin override → env → build-timerouting.defaultLocale. Values that aren't a shipped catalogue are ignored (regional tags likept-BRmap ontoptat match time, not here).i18n/request.ts– next-intl's fallback uses it.proxy.ts– next-intl bakesdefaultLocaleintocreateIntlMiddleware, so the proxy now builds one middleware per default-locale value seen (cached in aMap). Lets the admin change it without a restart./api/configexposesdefaultLocale;i18n/runtime-default-locale.tsmirrors it on the client (filled fromfetchConfig()) andlib/deep-links.tsuses it soas-neededURL prefixing agrees with the server.i18n/locale-names.ts– autonyms extracted fromLanguageSwitcherand shared with the new admin select..env.example, FEATURES.md. Also brought the supported-locale lists up to date (mn,nb,zh-TWwere missing).The static Lite build is untouched: it has no server, so it keeps using
NEXT_PUBLIC_DEFAULT_LOCALE.Tests
lib/admin/__tests__/default-locale.test.ts– configured locale wins; empty falls back to build default; unknown / regional values ignored.i18n/__tests__/runtime-default-locale.test.ts– client mirror starts at build default, follows the server value, drops invalid values.npm run typecheck,npm run lint,npx vitest runall green (the one failure locally is the pre-existing timezone-dependentbirthday-calendartest, unrelated).Notes for review
feat: resolve deep links to the local instance); itsisConnectorRouteexclusion inproxy.tsis preserved.