docs: React server components example and docs #1903
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
fromthemills
force-pushed
the
sc-docs-example
branch
from
56d9c8d
to
44d644e
Compare
andrii-bodnar
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1903 +/- ##
==========================================
- Coverage 76.65% 76.57% -0.08%
==========================================
Files 81 82 +1
Lines 2090 2092 +2
Branches 533 534 +1
==========================================
Hits 1602 1602
- Misses 375 377 +2
Partials 113 113 ☔ View full report in Codecov by Sentry. |
|
Hi @fromthemills, thanks a lot for the contribution! I just read through the PR and have some suggestions:
|
|
i quickly check the example, here is my first impressions:
|
@thekip Would you like this under Tutorials or Guides?
True it is not picked up by the extractor. Any suggestions for how we will "recommend" to do server only translations? Just using the instance? like: i18n._('Learn') |
|
What about creating a separate Tutorial page? As described in #1876 |
|
Hello and thanks for the PR! I already have have a good idea of what I wanted to cover and I was planning to make it a little more comprehensive:
@andrii-bodnar actually the locale switcher, as far as I can tell, should demonstrate that a full page reload is not needed (why do you think it should use full page reload?). However, when this all adds up, I fear it could take a lot of time to get aligned using the standard loop of [write code -> get a review -> address review], as each step takes time on one side or the other. So the way I see it we can (1) do the loop, or (2) merge this into a side branch of this repo, and I could take it from there and iterate toward a version that addresses the above. That way also your time and commits are reflected in the repo history. Just a suggestion - happy to do it either way. Thank you! :) |
fromthemills
force-pushed
the
sc-docs-example
branch
from
44d644e
to
eda630a
Compare
|
@thekip @andrii-bodnar I incorporated your feedback into the PR. Most notable changes:
@vonovak I would prefer to just merge this. I think it kind of covers most things. Maybe after the merge you can refine more? Future worksRight now for server components translations I use the t(i18n)`Home`Obviously this is not ideal but as @thekip mentioned import { TransServer } from '@lingui/macro'
import { setI18n } from '@lingui/react'And then maybe in a next version add some dynamic switching capabilities like the POC @thekip made Something to think about |
|
TY, this is much better, though I'd still prefer to have one example over two 🤔. edit: but I guess it's okay, sorry for showing up late with my comments. @fromthemills may I ask why you didn't use the server-side Trans, as you linked? TY :) btw the docs import TransNoContext but don't use it. (I'll do a more thorough review later). |
No worries. True... one example might be better. But the old example used
Nice catch, thank you. Removed them.
Good point. Mainly, I did not want to use the more "experimental" part of the POC with the I see possible future in that in a next version of Lingui React you might be able to setup a i18n "context" with Lingui in 2 ways:
Usage can than differ from client and server components // client components only
import { I18nProvider } from '@lingui/react'
import { Trans, t } from '@lingui/macro'
<I18nProvider> // sets client context and part of js bundle
{t`Hello`}
<Trans>Hello</Trans> // also part of js bundle
</I18nProvider>// client or server components
import { setI18n } from '@lingui/react/server'
import { Trans, t } from '@lingui/macro/server'
setI18n(...) // sets server context
{t`Hello`}
<Trans>Hello</Trans> // server only not part of js bundleAfter that we could than go one step further and create a new package called import { I18nProvider } from '@lingui/react'
import { setI18n } from '@lingui/react/server'
import { Trans, t } from '@lingui/macro/resolve'
setI18n(...) // sets server context
<I18nProvider> // sets client context
{t`Hello`}
<Trans>Hello</Trans> // Knows render context and switch to use "@lingui/react/server" or " '@lingui/react"
</I18nProvider>/** @type {import('next').NextConfig} */
module.exports = {
webpack: (config) => {
config.resolve.plugins.push(require('@lingui/rsc-webpack-resolver');
return config;
},
} |
fromthemills
force-pushed
the
sc-docs-example
branch
from
eda630a
to
2b1f3a4
Compare
|
I recently realized that neither There are two ways to fix that, one is explicitly add |
|
I'll need to double-check but for me, it seemed that using the LinguiTransRscResolver worked fine with extraction 🤔, when importing like this: I think it's fine to document using
I didn't do any alterations there so not sure what you have in mind 🤔 It's true that I didn't use the From your examples: Ideally we should only need My take is: we shouldn't add |
or extract a separate package, specifically for nextjs. I think if we will go with the way of |
I agree adding this to Lingui itself might be to contrived... the APIs are to unstable ATM.
Yeah, also leaning more in this direction of // next.config.js
const { withLingui } = require('@lingui/nextjs')
// sets up swc plugin and resolver
module.exports = withLingui({
// Other Next.js configuration goes here
});// page.js or layout.js
import { setI18n } from '@lingui/nextjs'
setI18n('en')The only thing I don't like about it is that you would not have access to the underlying primitives and you are relying on the resolver and swc plugin config (runtimeModules) to do the switching. Or maybe something like below is possible? I don't know... maybe to complicated. import { setI18n } from '@lingui/nextjs'
import { Trans, t } from '@lingui/nextjs/macro' // always uses get18n |
|
Firstly, guys, I want to say a big thank to all of you. I really appreciated that someone take contributions that seriously and willing to discuss and help. I'm lacking this "second opinion" for most of the time.
This thing is actually bothered me from the beginning. I used some non-documented internals of nextjs, which might be subject of change at any time. From other side the concept is neat and gives excellent DX. We can try to reach nextjs team and ask them to make this public or at least notify them that one of the libraries relaying on that and them what they think. Also, just to be on the same page want to share latest changes which may affect discussion here:
If the way with standardizing i'm thinking about a more radical solution, what if we allow a macro to process a user defined symbol? (this will not work with babel-macro-plugin, but will work in all other compilers, babel-plugin and swc) // config
macro: {
{
module: "@myApp/lingui-trans-rsc",
export: "TransRsc"
}
}
// will process
import {TransRsc} from "@myApp/lingui-trans-rsc"
<TransRsc>Hello</TransRsc>
// into
import {TransRsc} from "@myApp/lingui-trans-rsc"
<TransRsc id="<hash>" message="Hello"></TransRsc>This will unblock people to use macro with custom |
| config.module.rules.push({ | ||
| test: /\.po/, | ||
| use: [ | ||
| options.defaultLoaders.babel, |
There was a problem hiding this comment.
do we need this line? @lingui/loader should return a ready to use module for webpack
There was a problem hiding this comment.
No, your right it is not needed. Coppied it from an example.
| } | ||
| ``` | ||
|
|
||
| As you can see in the example above we slight modified the default `I18nProvider` with our own implementation. Here we pass the active message catalog as a prop. This is to avoid importing all catalogs inside the `I18nProvider` component itself and thus making them part of the client bundle. For the people who are wondering: Why we are not just passing an instance of `i18n` the original provider component? Server components can pass data to client components but the caveat is that this needs to be serializable data and `i18n` is not. |
There was a problem hiding this comment.
if we will go with "@lingui/nextjs" may be pre-made this provider in that package?
There was a problem hiding this comment.
Hmm... not 100% sure. One the one hand you would want the package to "batteries included" but you don't want to inflate the scope to much. Leaning more to "it should be included".
There was a problem hiding this comment.
at this point, I'm not for creating @lingui/nextjs package. Let's lay the groundwork first, try to be as universal as possible and see later about adding framework-specific packages?
website/docs/tutorials/nextjs.md
Outdated
|
|
||
| Finally an example of a locale switcher. | ||
|
|
||
| ```jsx |
I've checked my PoC once again, and yes, the |
|
One more thing to consider is Turbopack, the |
|
Created an issue in NextJs repo vercel/next.js#64244 |
Love the project ❤️
Dang 🫠
We could either incorporate this into
I really like this approach @thekip I added your changes. Also took the liberty to update the current nextjs example to
How do we see this PR? Do we merge this as is? It currently covers the basics without to much of the experimental stuff. And as I said before just using client component We could wait on #1909, to include TransNoContext example? |
|
Long story short, we can use What i'm thinking right now is how to correctly implement it in lingui. We can expose Features to consider:
I want to prepare another PoC with changes I mentioned to prove the ideas. |
|
Tested, that works, let's discuss public API, how i see that:
I believe it would be a good idea to include However, Next.js app router behavior is extremely weird. While SSR layouts executed after pages, but context inside react tree is flowing from layout to pages (means layout components executed before pages) That means for us that react context provider in Layout will be propagated to pages, but If we will not merge provider with Both options have pros and cons. I didn't find a way where we can set |
|
Global |
|
Available in the v4.10.0 🚀 @fromthemills could you please consider the new features in the example and the article? There is an example mentioned here - #1914 |
|
@andrii-bodnar @thekip Will incorporate the new changes in the docs |
| ```js | ||
| //next.config.js |
There was a problem hiding this comment.
| ```js | |
| //next.config.js | |
| ```js title="next.config.js" | |
And please do the same for other files
|
|
||
| type Props = { | ||
| locale: string | ||
| messages?: any |
There was a problem hiding this comment.
messages can be types like this:
import { type Messages } from "@lingui/core"
| children: React.ReactNode | ||
| } | ||
|
|
||
| export function I18nProvider({ locale, messages, ...props }: Props) { |
There was a problem hiding this comment.
I suggest this implementation:
export function LinguiClientProvider({
children,
initialLocale,
initialMessages,
}: {
children: React.ReactNode
initialLocale: string
initialMessages: Messages
}) {
const [i18n] = useState(() => {
return setupI18n({
locale: initialLocale,
messages: { [initialLocale]: initialMessages },
})
})
return <I18nProvider i18n={i18n}>{children}</I18nProvider>
}with the above, depending on how users set it up, the provider might end up re-rendering and if that happens, new i18n instance would be constructed, and all Trans client components would re-render.
There was a problem hiding this comment.
Would it be better to use a useMemo with [locale, messages] deps?
I don't really understand why it's became initial since, when language switched this provider would be called with new props
| Since version 13+ Next.js allows you to use [server components](https://nextjs.org/docs/app/building-your-application/rendering/server-components) using the app directory. Lingui can easily be used with server components. There are however some considerations to be made so we can make optimal use of server components and it's capabilities. | ||
|
|
||
| 1. We want to avoid shipping to much js to the browser if it is not needed | ||
| 2. We want to be able to easily distinguish between client component translations (part of the js bundle) and server component translations. |
| } | ||
| ``` | ||
|
|
||
| As you can see in the example above we slight modified the default `I18nProvider` with our own implementation. Here we pass the active message catalog as a prop. This is to avoid importing all catalogs inside the `I18nProvider` component itself and thus making them part of the client bundle. For the people who are wondering: Why we are not just passing an instance of `i18n` the original provider component? Server components can pass data to client components but the caveat is that this needs to be serializable data and `i18n` is not. |
There was a problem hiding this comment.
at this point, I'm not for creating @lingui/nextjs package. Let's lay the groundwork first, try to be as universal as possible and see later about adding framework-specific packages?
| if (locale === 'fr') { | ||
| return fr | ||
| } |
There was a problem hiding this comment.
as you add more languages, this pattern grows not nice, I suggest using an object
There was a problem hiding this comment.
at this point, I'm not for creating @lingui/nextjs package
This is no longer a proposition after discovering react-server export condition.
(for some reason it will not give me an ability to left a reply under the original comment)
|
|
||
| Since version 13+ Next.js allows you to use [server components](https://nextjs.org/docs/app/building-your-application/rendering/server-components) using the app directory. Lingui can easily be used with server components. There are however some considerations to be made so we can make optimal use of server components and it's capabilities. | ||
|
|
||
| 1. We want to avoid shipping to much js to the browser if it is not needed |
There was a problem hiding this comment.
| 1. We want to avoid shipping to much js to the browser if it is not needed | |
| 1. We want to avoid shipping too much js to the browser if it is not needed |
I also think that layouts are not executed for client-side route transitions, so basically I ended up wrapping all layouts and pages in |
|
Hi @fromthemills, do you need any help with finishing this PR? |
|
Hi! I'll be happy to take this over the finish line but I'm vacationing now, so I can look into this after 22/5, unless @fromthemills plans to do that 👍. |
|
Since there's no news on this, I'll get started on this soon, probably later this week. 👍 |
Description
Types of changes
Fixes # (issue)
Checklist