amannn / next-intl

🌐 Internationalization (i18n) for Next.js
https://next-intl-docs.vercel.app
MIT License
2.58k stars 236 forks source link

feat: `next-intl@3.22` #1391

Closed amannn closed 3 weeks ago

amannn commented 1 month ago

Update:


next-intl@3.22 is around the corner and already available as a prerelease:

npm install next-intl@canary

I'd be grateful if some users of next-intl could help to test out this version. I've got an extensive test suite, but it'd be great to get feedback on whether it works in all real-world cases. Please leave a note here if you've decided to try out the prerelease and let me know if everything works as expected!

As this is a minor release, there are no adaptions necessary to your app in order to continue working as before. However, two new APIs have been added that will supersede previously available ones that are now deprecated—see the linked PRs below for details.

Improvements

  1. https://github.com/amannn/next-intl/pull/1316
  2. https://github.com/amannn/next-intl/pull/1383
  3. https://github.com/amannn/next-intl/pull/1414
  4. https://github.com/amannn/next-intl/pull/1389
  5. https://github.com/amannn/next-intl/pull/1411
  6. https://github.com/amannn/next-intl/pull/1437

Next.js 15 compatibility

Adopting createNavigation and await requestLocale will get your app in shape for Next.js 15 to work without any warnings (see https://github.com/amannn/next-intl/issues/1375).

Related issues

Fixes https://github.com/amannn/next-intl/issues/444 Fixes https://github.com/amannn/next-intl/issues/1335 Resolves https://github.com/amannn/next-intl/issues/454 Resolves https://github.com/amannn/next-intl/issues/1268 Resolves https://github.com/amannn/next-intl/issues/1055 Addresses https://github.com/amannn/next-intl/discussions/785

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
next-intl-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 21, 2024 2:17pm
next-intl-example-app-router ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 21, 2024 2:17pm
next-intl-example-app-router-without-i18n-routing ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 21, 2024 2:17pm
ScreamZ commented 1 month ago

Hey @amannn! As promised here is the first set of feedback, I haven't reviewed your changes in terms of code yet, so I might be missing some implementation details. Once again, sorry for the big block of text!

Here are a few pieces of feedback

First I like the new design and the way you listen to the community :) I'm happy that I can now use various API without being within the middleware!

Just migrated my not-found.tsx root

➡️ from this ```tsx import { validateLocaleOrFallback } from "i18n/validate-locale-or-fallback"; import { getTranslations } from "next-intl/server"; export default async function GlobalNotFound() { const locale = validateLocaleOrFallback(); const t = await getTranslations({ locale }); return ( ```
➡️ to this ```tsx import { getLocale, getTranslations } from "next-intl/server"; export default async function GlobalNotFound() { const [locale, t] = await Promise.all([getLocale(), getTranslations()]); return ( ```

And this will also save a lot of other files. I haven't finished migrating things to test the new behavior, I'll keep this updated. Next one is my custom pattern with auth router https://github.com/amannn/next-intl/discussions/1170 that should become useless. Or just if you want to keep localized name routing.

[!NOTE] For the rest of this post, you'll see a lot of calls to my function validateLocaleOrFallback(locale?: string): AvailableLocales This function is mainly used to validate the locale or get it from somewhere else than URL. I'm pretty sure this is somehow what you do under the hood, wich leads me to a specific point below (keep reading ahah)

➡️ Here is the implementation ```tsx import { cookies, headers } from "next/headers"; import { AvailableLocales } from "./locales"; /** * Ensure locale or fallback if not provided. * A function to use most of time out of next-intl router context, as any function like `getLocale` will trigger a `notFound` invocation and cause unexpected behavior. */ export function validateLocaleOrFallback(locale?: string): AvailableLocales { if (!locale || !AvailableLocales.includes(locale as AvailableLocales)) { const cookiesList = cookies(); // Leverage https://nextjs.org/docs/pages/building-your-application/routing/internationalization#leveraging-the-next_locale-cookie locale = cookiesList.get("NEXT_LOCALE")?.value; if (locale) return locale as AvailableLocales; // I know this is going to be async soon ! const headersList = headers(); // Can be Accept-Language: fr-CH, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5 on safari worst case scenario locale = headersList.get("accept-language")?.split(",")[0]; // fr-CH, fr;q=0.9 => fr-CH locale = locale?.split("-")[0].toLocaleLowerCase(); // fr-CH => fr // Check if exists and valid (also stripe *) if (locale && AvailableLocales.includes(locale as AvailableLocales)) return locale as AvailableLocales; return AvailableLocales[0]; // Final fallback } return locale as AvailableLocales; } ```

Being able to pass generic for getLocale and useLocale

I keep my time casting the locale. But for method where we checked it in getRequestConfig already, this could be nice to have those method typed. Just like the translations keys.

Add precision whether requestLocale is not required to be awaited.

Also, I would rather pass it as a function like guessLocale. This way you can lazy load it. If you don't need it, it won't trigger any locale extraction. What do you think? Or are you already guessing values lazily with a promise-compliant object that triggers in .then?

notFound in layout

Should I rather do the notFound in metadata or in the page render ?

Below my old implementation still (I'll update this post over time)

import { UserNavTracker } from "@modules/navigation/components/UserNavTracker";
import { validateLocaleOrFallback } from "i18n/validate-locale-or-fallback";
import { Metadata } from "next";
import { getTranslations } from "next-intl/server";

type LocaleLayoutProps = Readonly<{
  children: React.ReactNode;
  params: { locale: string };
}>;

export async function generateMetadata({ params: { locale } }: LocaleLayoutProps): Promise<Metadata> {
  locale = validateLocaleOrFallback(locale); // I'm using it here ! hehe
  const t = await getTranslations({ locale, namespace: "SEO" });

  return {
    title: t("common.title"),
    description: t("common.description"),
    keywords: t("common.keywords"),
  };
}

export default async function LocaleLayout({ children }: LocaleLayoutProps) {
  return (
    <>
      <UserNavTracker />
      {children}
    </>
  );
}

Debugging Unable to findnext-intllocale because the middleware didn't run on this request. See https://next-intl-docs.vercel.app/docs/routing/middleware#unable-to-find-locale. ThenotFound()function will be called as a result.

I'm still experiencing this a lot, and this is hard to debug, considering the new paradigm can we improve this?

Guessing value

It's the topic about my validateLocaleOrFallback function. The naming is not best I think I should work on it to something like guessBestLocale.

Do you have the equivalent somewhere or can we consider adding this as an helper somewhere.

Why is that?

Because sometimes, especially with the current authjs beta, you cannot always relies on URL. Or sometimes you want non-i18n webhooks, like for oAuth redirection.

Using this, we can guess the locale, as a fallback on something else.

Not sure about the strategy of using a default locale, if we can first guess it from headers or cookies. The default value should be in last resort if the two first strategy fails. Unless you're as you explained managing you locale on you own way like user settings.

BEFORE

import { getRequestConfig } from "next-intl/server";
import { ArrayValues } from "type-fest";
import { AvailableLocales } from "./locales";

export default getRequestConfig(async ({ requestLocale }) => {
  let locale = await requestLocale;

  // Validate that the incoming `locale` parameter is valid
  if (!AvailableLocales.includes(locale as ArrayValues<typeof AvailableLocales>)) {
    // I WANT FALLBACK ON Cookie, then Accept language header, then default to the first locale
    locale = AvailableLocales[0];
  }

  return {
    locale,
    messages: (await import(`./messages/${locale}.json`)).default,
  };
});

AFTER

import { getRequestConfig } from "next-intl/server";
import { validateLocaleOrFallback } from "./validate-locale-or-fallback";

export default getRequestConfig(async ({ requestLocale }) => {
  const locale = validateLocaleOrFallback(await requestLocale);

  return {
    locale,
    messages: (await import(`./messages/${locale}.json`)).default,
  };
});

Here are my first check, I'll keep posting (this is a big markdown post, I should have split)

I'll try finish migrating soon, and try in production, If you're interested the website is live on https://www.supafriends.com (not on canary yet)

amannn commented 1 month ago

@ScreamZ Thanks again for the great feedback, and even more so for proposing the idea with the fallback locale in the first place! This came just at the right time since we have to make a switch towards an async API anyway for Next.js 15.

Promise.all

A side note, but regarding this:

const [locale, t] = await Promise.all([getLocale(), getTranslations()]);

While generally a good idea for API calls, there's no performance difference here. You can absolutely leave it like this to improve readability if you prefer:

const locale = await getLocale();
const t = await getTranslations();

I keep my time casting the locale.

Which cases do you have practically? But yes, I also made a note about this recently: https://github.com/amannn/next-intl/issues/1377.

Add precision whether requestLocale is not required to be awaited.

Hmm, not sure if I follow. It's the same kind of "getter" that e.g. await params is in Next.js 15. I.e., if you need to value, then await it, if you don't, then you don't.

There is no "guessing" happening here btw. However the following priorities are taken into account:

  1. Overrides like getTranslations({locale: …})
  2. A static locale provided via unstable_setRequestLocale
  3. A value that was matched by the middleware (which in turn is based on a list of priorities)

Should I rather do the notFound in metadata or in the page render ?

Either is fine and will work. While Next.js is expected to call these in parallel, you could try to investigate with some logging if one or the other is reliably called first. In case you have some data fetching, that could help to render the 404 page slightly faster if you abort the render before that kicks in.

Unable to find next-intl locale because the middleware didn't run on this request.

Is that after the update? Can you give an example of a case where you run into this? I'm also a bit curious since the error message is slightly different now. Since you're always returning a locale from getRequestConfig this should honestly not happen at all now. Probably worth digging a bit further into this if you can diagnose a case where this happens.

Guessing value

I'd recommend not "guessing" the locale. The locale should be a strict contract, with every page and every route that your application provides having a clear assumption of if it's called with a locale or not. For pages, it's the middleware's job to reliably provide this. For Route Handlers, my recommendation is still to provide it explicitly, e.g. via a locale search param. If it's still missing somewhere, the new fallback locale can be used, but this should be a conscious decision (e.g. a global language selection page at /, or a global 404 page).

Hope this helps, let me know if you have more questions!

ScreamZ commented 1 month ago

New use case as I replaced next-auth with custom lucia-auth + artic code.

For OAUth callback, I'm using a non-localized endpoint, this helps not having to add a new locale to authorized locale whitelist in the oAuth provider.

Therefore I would like to access my custom redirect to redirect to the proper page in case of oAuth error (localized)

Before 3.22 this wasn't possible as you cannot pass the locale yourself to redirect function. I'll try now with 3.22 canary if this correctly works with new capabilities.

ScreamZ commented 1 month ago

@amannn I can confirm that redirect (from next intl) doesn't care about what I'm returning from getRequestConfig.

[!NOTE] Edit Maybe the point 3 of https://github.com/amannn/next-intl/pull/1316 is related, I haven't changed my createNavigation yet. EDIT2: New API (Damn this is soooo coool, good job !) forcing to pass locale to redirect is doing the job, no more issues !

import { getRequestConfig } from "next-intl/server";
import { validateLocaleOrFallback } from "./validate-locale-or-fallback";

export default getRequestConfig(async ({ requestLocale }) => {
  const locale = validateLocaleOrFallback(await requestLocale);

  console.log("getRequestConfig", locale);
  return {
    locale,
    messages: (await import(`./messages/${locale}.json`)).default,
  };
});
export const config = {
  // Match only internationalized pathnames, this cannot be used with code so anytime a locale is used, the locale must be hardcoded here
  matcher: ["/", `/(en|fr|de|es|it)/:path*`, `/slice-simulator`],
};

this is voluntarily out of the middleware as i don't want auto-adding /fr or other to route.

// In my route handler path app/(not-localized)/auth/callback/[provider]/route.ts
    const t = await getTranslations();

      // Report not translated error code
      if (!t.has(`Auth.error.${code}` as any)) {
        await reportLogToDiscord("Authentication", "Not translated error", {
          "error.json": { e, code, provider },
        });
      }

      return redirect({ pathname: "/authentication/sign-in", query: { error: code } });

with logs

getRequestConfig fr
🪵 [Discord Log] Authentication: Not translated error

Unable to find `next-intl` locale because the middleware didn't run on this request. See https://next-intl-docs.vercel.app/docs/routing/middleware#unable-to-find-locale. The `notFound()` function will be called as a result.

 GET /auth/callback/google?state=1lpy2j_T0p9m9mrO8iXrqFVzs_TI5Rj

I haven't tested recently with other API, but I'm pretty sure that links also don't care about request config. or getLocale (or others).

Isn't possible to tell that for all server API getRequestConfig is master for having the request language and context ?

ScreamZ commented 1 month ago

Any ETA for this release ? I'm expecting to use this canary in production :)

amannn commented 1 month ago

@ScreamZ Cool to hear that things are progressing with your app! Do I understand correctly based on your edit that you got everything working now? Or is there any issue left to resolve?

Regarding ETA: I'm currently working on a follow-up for https://github.com/amannn/next-intl/pull/1414 in https://github.com/amannn/next-intl/pull/1417. Apart from that, there are currently no issues known regarding other features added in this PR. What I am waiting for though is Next.js publishing a new v15 RC, so I can confirm that we're compatible with their latest changes. With Next.js Conf coming up next week, I think that should be soon!

ScreamZ commented 1 month ago

@ScreamZ Cool to hear that things are progressing with your app! Do I understand correctly based on your edit that you got everything working now? Or is there any issue left to resolve?

Regarding ETA: I'm currently working on a follow-up for #1414 in #1417. Apart from that, there are currently no issues known regarding other features added in this PR. What I am waiting for though is Next.js publishing a new v15 RC, so I can confirm that we're compatible with their latest changes. With Next.js Conf coming up next week, I think that should be soon!

Everything (so far) is working for now.

Yeah I think they will do some announcement on Next conf

Thanks !

nhducit commented 1 month ago

I'm testing out the canary version but still getting this error

 In route /[locale] a header property was accessed directly with `headers().get('X-NEXT-INTL-LOCALE')`. `headers()` should be awaited before using its value. Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis

version

"next-intl": "3.22.0-canary.0",
"next": "v15.0.0-canary.191",

I guess we need to wait for https://github.com/amannn/next-intl/issues/1375

amannn commented 1 month ago

@nhducit #1375 is implemented in this PR via two new APIs that we're added (see "Next.js 15 compatibility" in the PR description above). Can you try to upgrade to those APIs to verify if this gets rid of the warnings for you?