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

Geting rid of setRequestLocale #663

Open
amannn opened this issue Nov 24, 2023 · 34 comments
Open

Geting rid of setRequestLocale #663

amannn opened this issue Nov 24, 2023 · 34 comments
Labels
area: performance enhancement New feature or request

Comments

@amannn
Copy link
Owner

amannn commented Nov 24, 2023

As mentioned in the static rendering docs, setRequestLocale is intended as a stopgap solution to be removed at some point. I've started a discussion for the relevant feature on the Next.js side here: vercel/next.js#58862

If you'd like to help simplify the usage of next-intl, please join the discussion there and upvote the comment!

@amannn amannn added enhancement New feature or request unconfirmed Needs triage. labels Nov 24, 2023
@amannn amannn pinned this issue Nov 24, 2023
@amannn amannn removed the unconfirmed Needs triage. label Nov 24, 2023
@xie392
Copy link

xie392 commented Dec 12, 2023

image
Some components need to use "use client", and I want to output static files. There is a conflict between the two. How should I solve it?

@amannn
Copy link
Owner Author

amannn commented Dec 12, 2023

You don’t need to call the function in client-only pages.

amannn added a commit that referenced this issue Dec 21, 2023
…and update docs to suggest validating the locale in `i18n.ts` (#742)

Users are currently struggling with errors that are caused by these two
scenarios:
1. An invalid locale was passed to `next-intl` APIs (e.g.
[#736](#736))
2. No locale is available during rendering (e.g.
[#716](#716))

**tldr:**
1. We now suggest to validate the incoming `locale` in
[`i18n.ts`](https://next-intl-docs.vercel.app/docs/usage/configuration#i18nts).
This change is recommended to all users.
2. `next-intl` will call the `notFound()` function when the middleware
didn't run on a localized request and `next-intl` APIs are used during
rendering. Previously this would throw an error, therefore this is only
relevant for you in case you've encountered [the corresponding
error](https://next-intl-docs.vercel.app/docs/routing/middleware#unable-to-find-locale).
Make sure you provide a relevant [`not-found`
page](https://next-intl-docs.vercel.app/docs/environments/error-files#not-foundjs)
that can be rendered in this case.

---

**Handling invalid locales**

Users run into this error because the locale wasn't sufficiently
validated. This is in practice quite hard because we currently educate
in the docs that this should happen in [the root
layout](https://next-intl-docs.vercel.app/docs/getting-started/app-router#applocalelayouttsx),
but due to the parallel rendering capabilities of Next.js, potentially a
page or `generateMetadata` runs first.

Therefore moving this validation to a more central place seems
necessary. Due to this, the docs will now suggest validating the locale
in `i18n.ts`. By doing this, we can catch erroneous locale arguments in
a single place before e.g. importing JSON files.

The only edge case is if an app uses no APIs of `next-intl` in Server
Components at all and therefore `i18n.ts` doesn't run. This should be a
very rare case though as even `NextIntlClientProvider` will call
`i18n.ts`. The only case to run into this is if you're using
`NextIntlClientProvider` in a Client Component and delegate all i18n
handling to Client Components too. If you have such an app, `i18n.ts`
will not be invoked and you should validate the `locale` before passing
it to `NextIntlClientProvider`.

**Handling missing locales**

This warning is probably one of the most annoying errors that users
currently run into:

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

The various causes of this error are outlined in [the
docs](https://next-intl-docs.vercel.app/docs/routing/middleware#unable-to-find-locale).

Some of these cases should simply be 404s (e.g. when your middleware
matcher doesn't match `/unknown.txt`), while others require a fix in the
matcher (e.g. considering `/users/jane.doe` when using `localePrefix:
'as-necessary'`).

My assumption is that many of these errors are false positives that are
caused by the `[locale]` segment acting as a catch-all. As a result, a
500 error is encountered instead of 404s. Due to this, this PR will
downgrade the previous error to a dev-only warning and will invoke the
`notFound()` function. This should help in the majority of cases. Note
that you should define [a `not-found`
file](https://next-intl-docs.vercel.app/docs/environments/error-files#not-foundjs)
to handle this case.

I think this change is a good idea because if you're using
`unstable_setRequestLocale` and you have a misconfigured middleware
matcher, you can provide any kind of string to `next-intl` (also
`unknown.txt`) and not run into this error. Therefore it only affects
users with dynamic rendering. Validating the locale in `i18n.ts` is the
solution to either case (see above). Also in case something like
[`routeParams`](#663) gets
added to Next.js, the current warning will be gone entirely—therefore
tackling it from a different direction is likely a good idea.

The false negatives of this should hopefully be rather small as we
consistently point out that you need to adapt your middleware matcher
when switching the `localePrefix` to anything other than `always`.
Dev-only warnings should help to get further information for these
requests.

---

Closes #736
Closes #716
Closes #446
@AhmedBaset
Copy link
Contributor

I'm curious about whether calling setRequestLocale at the top level app/[locale]/layout.tsx will apply to the whole app or not?

When exporting generateStaticParams at the layout all sub routes are affected by it.

@amannn
Copy link
Owner Author

amannn commented Jan 9, 2024

Unfortunately not, please see the corresponding docs: Add unstable_setRequestLocale to all layouts and pages. It might work in some cases, but there's a race condition. To work reliably, you should add it to every layout and every page that you wish to enable static rendering for.

@amannn
Copy link
Owner Author

amannn commented May 1, 2024

We'll probably introduce support for providing a locale to next-intl as an alternative to having to use the middleware to negotiate this (see #1017). If your app only supports a single locale, this will enable static rendering without workarounds.

@amannn
Copy link
Owner Author

amannn commented May 13, 2024

A realization while working on #1017: We currently support a defaultLocale that is used as a fallback when matching locales in the middleware. However, the defaultLocale only applies to this particular case.

Developers have previously asked why the defaultLocale isn't used when using next-intl APIs outside of the [locale] folder (see e.g. #1067, but also others).

In case the developer could manage the reading of the locale from params manually in i18n.ts, this would also enable returning a fallback locale for cases like this.

Ideally, something like this would be possible:

import {notFound} from 'next/navigation';
import {getRequestConfig} from 'next-intl/server';
import {getParams} from '???';
 
// Can be imported from a shared config
const locales = ['en', 'de'];
 
export default getRequestConfig(async () => {
  let {locale} = getParams();
  if (!locale) locale = 'en';

  // Validate that the incoming `locale` parameter is valid
  if (!locales.includes(locale as any)) notFound();
 
  return {
    locale,
    messages: (await import(`../messages/${locale}.json`)).default
  };
});

@vordgi
Copy link

vordgi commented May 16, 2024

Hello. I recently used next-intl in a job project and now I've rolled it back. As I've worked on such a thing I know the importance of feedback)

A bit late comment, just to add to the value of this issue)

setRequestLocale loses context. At random moments. There are 2 pages - one works perfectly with translations, the other loses them. Both have a similar structure, both use client and server components, client contexts on top. Loses in a random place. That is, three component levels have translations, but everything below in the tree lost, and they were correctly passed to the clients (I initialize NextIntlClientProvider at the root of the page).

I have a feeling that it breaks when it render some clients components (but I can't confirm this in any way). I saw somewhere recommendation to use my context - it has the same problem. Apparently, there are something of component rendering in React.

Although in the case of my context, you can set the build to one process and everything will be correct, and the context in case of something is lost immediately on the entire page. So the reasons are different.

However, I see an update came out these days, that locale can be set the way you want to (where would I get the getParams method 🤔😁) - thank you, will try it.

Overall, thanks for the package, cool API!

@amannn
Copy link
Owner Author

amannn commented May 16, 2024

Hey @vordgi! Nice to hear from you here, been following your work with nimpl-getters! :)

Can you by chance provide a reproduction where you observe that the context is lost? The important part for setRequestLocale to work correctly is that it's added to each and every layout and page where you're using APIs from next-intl. It should receive the locale from params as soon as rendering starts and before any APIs from next-intl are called.

Are you using any more exotic features from Next.js? E.g. I could imagine that PPR could potentially conflict with this.

Btw. in case you're interested, I had a look at what it'd take for native support from Next.js for providing a "server context" yesterday: vercel/next.js#58862 (comment). Leave a comment there if you have additional context!

@vordgi
Copy link

vordgi commented May 16, 2024

The parameters have been added everywhere. I will try to create an repro, but basically, it looks like this:

const Page: React.FC<{ params: { lang: string } }> = async ({ params }) => {
	if (!validLocales.includes(params.lang)) return notFound();

	unstable_setRequestLocale(params.lang);
	const messages = await getMessages();

	return (
		<NextIntlClientProvider messages={messages}>
			{/* Client component with context.provider inside */}
			<ContextProvider value={{key1: [],key2: ''}}>
				<script type="application/ld+json" dangerouslySetInnerHTML={{ __html: JSON.stringify('...')}} />
				{/* Asynchronous server component, translations are only at the top level of this component, below they are lost */}
				<Layout>
					{/* server component whithout translates */}
					<Block1 />
				</Layout>
			</ContextProvider>
		</NextIntlClientProvider>
	);
};

Yes, there are indeed risks with PPR. But I think that if functionality is unavailable in part of next.js - just block the functionality in that part. That is, I will add the error "Do not use this in PPR". I hope such an opportunity will be. But in fact, with the current structure of next.js, it is very difficult to find out what the component is inside and it is difficult to rewrite this logic (I tried).

I am now looking at the possibility of adding the ability to create a page context. But still only at the beginning of the research

@vordgi
Copy link

vordgi commented May 17, 2024

Wish me luck)

getPageContext PR

@amannn
Copy link
Owner Author

amannn commented May 18, 2024

Regarding PPR: On a second thought, maybe this is not an issue at all, since PPR is about pre-rendering as much as possible statically with inner parts remaining dynamic. Dynamic rendering was always the easy part since we can just read a header from the middleware there. Let's see what Next.js 14 brings next week, maybe there are news in regard to PPR and we could do a few first tests.

getPageContext PR

Oh interesting, thanks for looking into this! As far as I understand the proposal, this would still require providing the locale for every page where you intend to read it, right? In that regard the ergonomics would be similar to unstable_setRequestLocale.

I think ideally Next.js should really have a built-in capability to read params deeply in RSC.

@vordgi
Copy link

vordgi commented May 18, 2024

In Next.js, you can add parameters to their internal context and then read it the same way I read in getPathname. But Next.js stubbornly doesn't want to go that way. So I'm trying to offer them alternatives.

Yes, it will need to be added for each page, it's something comparable to getStaticProps before, but you can read from anywhere. In general, this is close to Next.js API and I hope to push some solution.

@amannn
Copy link
Owner Author

amannn commented May 18, 2024

Yep, definitely a good idea to discuss which options we have! I think a tricky part about the story with params is that they can be different per layout/page. A single render might consist of multiple layouts that render and a single page. Theoretically there could be different params at every level (e.g. /[locale]/[tenant]/page.tsx).

@vordgi
Copy link

vordgi commented May 18, 2024

Since they want to render the layout separately (this needs to be technically done at first because it works differently now) - I think in the layout the get-params functionality should be blocked or fixed with params higher in the tree. This is logical in such a situation - it was assembled as a segment once - where it lies.

@AhmedBaset
Copy link
Contributor

I think in the layout the get-params functionality should be blocked or fixed with params higher in the tree.

Currently, you can already read params in the layout via the params prop. There would be no need to call getParams there. If you did, it should return the same value as the prop.

@vordgi
Copy link

vordgi commented May 18, 2024

Decided to make a PR with the implementation of get-params (what I talked about above, in the basic version - a very simple change).

@vordgi
Copy link

vordgi commented May 18, 2024

Currently, you can already read params in the layout via the params prop

Yes, but what about components 10 levels deeper. They now have no idea - they are inside layout or page. And with the current next.js architecture, it's unreal to find out. So I think next.js need to render layout separately and then we can understand what is being rendered and depending on it, return the necessary params or block the API (although it looks like there shouldnt be any problems with issuance). But it's still not clear how it would be with PPR, yes...

@vordgi
Copy link

vordgi commented May 20, 2024

Hello @amannn. I have to say sorry for incorrect conclusions. The cache works well.

The problem was that I used setRequestLocale in not-found (and there defined it dynamically or gave a default). And this could overwrite what is written at the page level.

The most annoying thing is that I knew that <NotFound /> is rendered with the page.

I rewrote it in not-found on prop drilling and everything works well.

So, in general, it is always necessary to consider that the cache, unlike the context (and AsyncLocalStorage), is precisely a global storage. And since Layout, NotFound, Error, and Page are rendered as a whole, such situations can arise when there are discrepancies.

@suitux
Copy link

suitux commented Jul 3, 2024

Any update on getting rid of this?

@darthmaim
Copy link

How about writing a SWC plugin to automatically inject unstable_setRequestLocale into all layouts and pages?

Then people would just have to add it to experimental.swcPlugins in the next.config.js (or it could be added by withNextIntl). The plugin should ignore pages/layouts that use "use client" or are not under the route /[locale]. The name of the prop should be configurable to support /[lang], /[language], ... and default to locale (or maybe to ['locale', 'lang', 'language'] to work with all 3). The plugin would also have to detect the routes in route groups (/(test)/[locale]), parallel routes, intercepting routes, ....

@amannn
Copy link
Owner Author

amannn commented Jul 4, 2024

Certainly an interesting idea! I don't have experience with writing SWC plugins, but if there's a community effort to build this as a separate package I'd be happy to follow along and potentially explore adding it to the docs or if the approach proves to work well, we could consider adding it to the core. I'm unsure currently if there are edge cases, so this might need so experimentation.

The ideal solution would still require support from the Next.js side I believe: vercel/next.js#58862. But if we can improve the current workaround for the time being, I'd be all for it.

@amannn
Copy link
Owner Author

amannn commented Jul 10, 2024

Here's an SWC plugin that could be used for inspiration: css-variable/swc (usage in Next.js)

@nevnein
Copy link

nevnein commented Aug 5, 2024

This is only tangentially related, but I also feel it does not deserve a separate issue: if you have a root layout outside [locale] eg. to render a global non-localized not-found as specified in the docs , then Next will opt-out of static rendering on every page, since the root layout does not (and cannot) have the mandatory unstable_setRequestLocale call.
I don't think there's a way to solve this without removing the root layout altogether, does anyone have any ideas?

@WenLonG12345
Copy link

This is only tangentially related, but I also feel it does not deserve a separate issue: if you have a root layout outside [locale] eg. to render a global non-localized not-found as specified in the docs , then Next will opt-out of static rendering on every page, since the root layout does not (and cannot) have the mandatory unstable_setRequestLocale call. I don't think there's a way to solve this without removing the root layout altogether, does anyone have any ideas?

I am having the same issues with you. Once I remove the layout.tsx and not-found.tsx that outside [locale]. next build command is successfully built. Any ideas?

@ixartz
Copy link

ixartz commented Oct 22, 2024

I'm curious about this issue, will it be addressed by Next.js 15 or Next-intl 4?

@amannn
Copy link
Owner Author

amannn commented Oct 23, 2024

@ixartz Please see the corresponding section in the next-intl 3.22 blog post on setRequestLocale.

@ixartz
Copy link

ixartz commented Oct 23, 2024

@amannn Thank you so much for sharing it, I'll read carefully https://next-intl-docs.vercel.app/blog/next-intl-3-22, it seems I totally miss this blog post where you share a lot of information.

@amannn
Copy link
Owner Author

amannn commented Oct 28, 2024

A note from the discussion at #1474: Even if we get rid of setRequestLocale, maybe the locale that getRequestConfig receives will still stick around. The one rare case seems to be that you want to pass an override for an app that renders content from multiple locales at the same time. Currently, requestLocale conflates reading from an override and the middleware (ref), therefore also reading from headers for the case were you'd only want to read an override. Maybe something to consider here for the future …

Another thought: For easier getting started, if we could read params deeply, could we get started with a middleware matcher of /?

@amannn
Copy link
Owner Author

amannn commented Oct 31, 2024

Finally some news: https://x.com/sebmarkbage/status/1851728589240115459.

Not much for now, but very happy to hear that something is coming.

This was referenced Nov 5, 2024
@reiv
Copy link

reiv commented Nov 8, 2024

@amannn can you share what that Tweet said? Looks like Seb recently nuked his account.

@amannn
Copy link
Owner Author

amannn commented Nov 8, 2024

@reiv The tweet just mentioned that the Next.js team is working on something that will address accessing "global" params like [locale] deeply in RSC.

Here's another mention from Jimmy Lai:

Screenshot 2024-11-08 at 15 00 24

@amannn
Copy link
Owner Author

amannn commented Nov 15, 2024

It's happening! vercel/next.js#72837 👀

@apostolos
Copy link

It's happening! vercel/next.js#72837 👀

I'm confused. I didn't expect rootParams to be async. How does this solve vercel/next.js#71927 ?

Unless... Sebastian mentioned something about being able to suspend above <html>. I don't how this will be possible but I guess we'd have to wait and see.

@amannn
Copy link
Owner Author

amannn commented Nov 15, 2024

Have you seen the comment below from Sebastian?

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

No branches or pull requests