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(next-international): add defaultLocale config option to createI18nServer #365

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zfm-lucaschultz
Copy link

Hi, thank you for the wonderful package.

We have started using it in our project and encountered a small edge case. Instead of placing the entire app into an app/[locale] folder, we determine the locale by fetching the user in the middleware, checking the preferred language, and then using a modified version of your middleware to set the cookie and header values. While I believe this scenario is common, for now, we are using it incorrectly 😅

This situation has led to an interesting edge case. Our app has almost no static routes because we rely on headers extensively for fetching. Consequently, we do not invoke setStaticParamsLocale at the top of our pages. However, during app building, Next.js assumes that all routes without parameters are static, attempts to prerender them, and only treats the route as dynamic upon encountering a headers or cookies call. Unfortunately, in practice, it does not reach this stage as we encounter an error before that point:

Error: Could not find locale while pre-rendering page, make sure you called `setStaticParamsLocale` at the top of your pages

To address this, I propose introducing a defaultLocale on the server for prerendering if setStaticParamsLocale was not called. Alternatively, defaultStaticLocale may be a more suitable option.

Copy link

vercel bot commented Feb 26, 2024

@zfm-lucaschultz is attempting to deploy a commit to the Tom Labs Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Owner

@QuiiBz QuiiBz left a comment

Choose a reason for hiding this comment

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

Interesting use-case, but I don't quite understand why this change is needed.

Next.js assumes that all routes without parameters are static

That's only true if you're not using any dynamic function like headers() / cookies(), which we are using if a page doesn't call setStaticParamLocale()

Could you share your middleware code?

@lucaschultz
Copy link

Hi,

Could you share your middleware code?

Sure thing! Here is a minimal example that reproduces the problem I was describing. I added a description of what I think is happening to the readme. I hope this helps somewhat in understanding what I meant. 🙂

It also implements the middleware/route structure like we are using it with next-international. If I haven't missed anything glaringly obvious, it's pretty straightforward to adapt your library to our use case. In my opinion, the only thing missing is exporting addLocaleToResponse and documenting how to set it up.

@QuiiBz
Copy link
Owner

QuiiBz commented Mar 16, 2024

Oh I see the issue you're running into! The thing is that you have to move all pages inside a [locale] folder (dynamic segment). Sorry for the delay btw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants