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.js 13 RSC integration #139

Closed
wants to merge 6 commits into from
Closed

feat: Next.js 13 RSC integration #139

wants to merge 6 commits into from

Conversation

amannn
Copy link
Owner

@amannn amannn commented Oct 27, 2022

Things to figure out

  • How to provide the context on the server side?
  • How to get the locale? Should we parse this ourselves since i18n routing is gone (ref 1, ref 2) ?
  • Should messages for the server- and client side be separately provided?
  • Maybe we need to merge messages now when providers are nested?
  • Support the same i18n config that Next.js currently has?
  • Potential new features
    • Locale negotiation (middleware? API inspiration)
    • Saving the locale to a cookie
  • Should we utilize react-server exports?

Afterwards

  • Update docs
  • Update examples

@vercel
Copy link

vercel bot commented Oct 27, 2022

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

Name Status Preview Updated
next-intl ✅ Ready (Inspect) Visit Preview Nov 22, 2022 at 4:26PM (UTC)

amannn added a commit that referenced this pull request Oct 27, 2022
Note that this is a compatibility release that allows to use `next-intl` in Next.js 13 apps. It's still advised to use `next-intl` via the `pages` folder, usage in the `app` folder with Server Components is not possible currently (this will be part of a separate release based on #139).
@amannn amannn changed the title Next.js 13 RSC integration feat: Next.js 13 RSC integration Nov 2, 2022
export default function RootLayout({children, ...rest}: Props) {
console.log(rest);

// How to get this from the URL?
Copy link

Choose a reason for hiding this comment

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

Maybe wrap all pages under optional [[locale]] then get using params

https://beta.nextjs.org/docs/routing/defining-routes#example

Copy link
Owner Author

Choose a reason for hiding this comment

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

Hey and thank you for your interest! Yep, that seems to work, I've experimented with it here:

export default function Index({params: {locale}}: Props) {
// TODO: Validate locale or redirect to default locale
return <p>Hello {locale}</p>;
}

I think that part is doable, the part where I currently don't see an ergonomic solution is how global configuration like messages can be passed to the library (see the topic about context in the PR description).

Copy link

@fdarian fdarian Nov 11, 2022

Choose a reason for hiding this comment

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

I've tried some workarounds,
1, Using singleton — Failed because for RSC, initializing on root layout requires 2 render to assign the messages object.
2. Using third-party state manager like zustand — Failed because of the same reason as number 1.

Probably the solution is gonna be around client component. To use in a server component we could provide utility component like <T key='dashboard.hi' />. Performance-wise, next will also server-render client components so the downside is on developer experience. But won't be much I think.

ps: I can't find any documentation for createServerContext, but I just try to use it regardless the type-error and it seems like it works without sacrificing DX. However, we need to refactor functions out of the context.

import type {NextRequest} from 'next/server';
import i18n from './i18n';

// Somehow not invoked?

Choose a reason for hiding this comment

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

From a quick glance at the example WIP, the middleware doesn't seem to fire unless there is a "pages" folder as well. After that, it fires on both "pages" and "app" folders.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Oh cool, thanks for figuring this out! Seems like a bug on the Next.js side.

@daniel-koudouna
Copy link

daniel-koudouna commented Nov 18, 2022

Hi, thanks for your work. We had decided to use next-intl for a project a while ago, and we are in the process of slowly migrating to next 13.

I'm not sure if this will be of any use, but I've added a working (toy) example of a header-based solution: https://github.com/daniel-koudouna/next-intl/tree/feat/next-13

Some things to note:

  • I eliminated the need for a root [locale] path, as we can just find it from the middleware and rewrite it accordingly.
  • I added back the domains mappings, which can also be taken from the middleware

I'm not well versed with react internals by any means, this is just another experiment. Let me know what you think, I can open a PR to this branch if it helps.

@amannn
Copy link
Owner Author

amannn commented Nov 21, 2022

Hey @daniel-koudouna and thanks a lot for chiming in!

This branch is a really raw draft at this point, mostly for experimenting. I'm currently using Next.js internal APIs in Storage.tsx, which could break in a patch release. I'd like to get rid of this in favour of a proper solution.

Using headers for detecting the locale is a good idea, but we'd also need some storage for the messages and other config. Otherwise we'd have to work around it, but if possible, I'd really like to continue using the current API. I really hope that the Next.js/React will consider an API to help with this (see also vercel/next.js#42301 (comment)).

That is a bit the dealbreaker currently in my prototype and I'd like to find a better solution before moving forward. I think the other parts should be manageable.

But since Next.js is moving i18n functionality out of the core, probably this library will also participate in locale negotiation in the future. Would be really cool to get input from you when there's a first working prototype! Also great that you mention domains, probably we should support the same configuration as Next.js currently does, for an easy migration.

@daniel-koudouna
Copy link

Hey @daniel-koudouna and thanks a lot for chiming in!

This branch is a really raw draft at this point, mostly for experimenting. I'm currently using Next.js internal APIs in Storage.tsx, which could break in a patch release. I'd like to get rid of this in favour of a proper solution.

Using headers for detecting the locale is a good idea, but we'd also need some storage for the messages and other config. Otherwise we'd have to work around it, but if possible, I'd really like to continue using the current API. I really hope that the Next.js/React will consider an API to help with this (see also vercel/next.js#42301 (comment)).

That is a bit the dealbreaker currently in my prototype and I'd like to find a better solution before moving forward. I think the other parts should be manageable.

But since Next.js is moving i18n functionality out of the core, probably this library will also participate in locale negotiation in the future. Would be really cool to get input from you when there's a first working prototype! Also great that you mention domains, probably we should support the same configuration as Next.js currently does, for an easy migration.

No worries, happy to help if I can, although I'm not at all familiar with React internals. I found the headers-based solution pretty easy to add domains to.

@amannn
Copy link
Owner Author

amannn commented Dec 8, 2022

Closed in favour of #149

@amannn amannn closed this Dec 8, 2022
@amannn amannn deleted the feat/next-13 branch December 19, 2022 10:21
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