search

nuxt-modules / i18n

I18n module for Nuxt
https://i18n.nuxtjs.org
MIT License
1.75k stars 483 forks source link

useLocaleHead prerenders properly. useSwitchLocalePath does not #2798

Closed Pijuli closed 8 months ago

Pijuli commented 9 months ago

Environment

@nuxtjs/i18n@8.1.1

Reproduction

I'll be glad to make it if there's not a clue

Describe the bug

Having a dinamic route page and a language switcher, while useLocaleHead generates the right urls, useSwitchLocalePath does not. It gets, what I assume, is the default slug before updating the slug with useSetI18nParams.

That leads into the crawler having wrong urls and forcing me to ignore prerender on nitro block. While this do a proper build it does not escalate on my application and is just for debug purposes. I will be dealing with thousand urls soon.

Once the server is started, this urls are fixed, but they do appear wrong on the dist files

Related?

Additional context

No response

Logs

No response

github-actions[bot] commented 9 months ago

Would you be able to provide a reproduction? 🙏

More info ### Why do I need to provide a reproduction? Reproductions make it possible for us to triage and fix issues quickly with a relatively small team. It helps us discover the source of the problem, and also can reveal assumptions you or we might be making. ### What will happen? If you've provided a reproduction, we'll remove the label and try to reproduce the issue. If we can, we'll mark it as a bug and prioritise it based on its severity and how many people we think it might affect. If `needs reproduction` labeled issues don't receive any substantial activity (e.g., new comments featuring a reproduction link), we'll close them. That's not because we don't care! At any point, feel free to comment with a reproduction and we'll reopen it. ### How can I create a reproduction? We have a couple of templates for starting with a minimal reproduction: 👉 [Reproduction starter (v8 and higher)](https://stackblitz.com/fork/github/BobbieGoede/nuxt-i18n-starter/tree/v8) 👉 [Reproduction starter (edge)](https://stackblitz.com/fork/github/BobbieGoede/nuxt-i18n-starter/tree/edge) A public GitHub repository is also perfect. 👌 Please ensure that the reproduction is as **minimal** as possible. See more details [in our guide](https://nuxt.com/docs/community/reporting-bugs/#create-a-minimal-reproduction). You might also find these other articles interesting and/or helpful: - [The Importance of Reproductions](https://antfu.me/posts/why-reproductions-are-required) - [How to Generate a Minimal, Complete, and Verifiable Example](https://stackoverflow.com/help/mcve)
Pijuli commented 9 months ago

@BobbieGoede there you go

https://stackblitz.com/~/github.com/Pijuli/i18nissue https://github.com/Pijuli/i18nissue

While dev works properly and generates the right urls, npm run generate does not. Routes while in dev mode are: / /es /en /es/unapagina/espanol /somepage/english

While when doing npm run generate, these are the parsed pages: / /es /en /es/unapagina/espanol :heavy_check_mark: /somepage/english :heavy_check_mark: /somepage/espanol :x: /es/unapagina/english :x:

:x: Those pages do not "exist" and shouldn't have been generated. If you look at the dist folder you will see that they come from the language switcher. The slug is not properly rendered and the crawler gets them. Once the app is opened in the browser, the links are fixed. What am I missing? Do I have any missconfiguration of the SSR?

In this example, there's no "problem" but to have more statics that won't be ever navigated to. But in my app, when I try to retrieve info from those urls, it breaks

Workaround: 

nitro: {
  prerender: {
    ignore: [
      '/es/unapagina/english',
      '/somepage/espanol'
    ]
  }
}

but this can't be scaled in a +500 urls

Pijuli commented 9 months ago

There's a new commit in my repo "fixing the issue". Moving NuxtPage over the component that renders switchLocalePath(x), in my case LanguageSwitcher, seems to render the urls properly. This does not make any sense for me. Probably a hint.

Pijuli commented 9 months ago

Ok, finally found a "workaround"(?). Use <client-only> in the language switcher. I've updated the repo. What's the reason behind this?

BobbieGoede commented 9 months ago

@Pijuli

There's a new commit in my repo "fixing the issue". Moving NuxtPage over the component that renders switchLocalePath(x), in my case LanguageSwitcher, seems to render the urls properly. This does not make any sense for me. Probably a hint.

Use in the language switcher. I've updated the repo. What's the reason behind this?

This behaviour is described in the docs as a notice here though when I wrote that I didn't consider how this would work for prerendering. In short: there's no reactivity during SSR and the lang switcher is rendered before setI18nParams is called, during client-side hydration the lang switcher is updated.

Head tags do get updated, I believe unhead replaces rendered tags just before the server sends out its response, this makes essentially makes it reactive even though it was already rendered. I'm still considering if we could add something similar specifically for language switchers URLs, I imagine it's not as predictable as head tags so this might be difficult.

I didn't expect wrapping the language switcher in <client-only> would work for prerendering. If this doesn't cause other issues I might add it to the docs, it sounds like a good/easy workaround and I'm sure others are running into the same problem.

Pijuli commented 9 months ago

Hey, @BobbieGoede,

I'm far from knowing the insides of this, so I can probably be of little to no help here, but, talking about head tags, they got properly rendered in the statically generated html head tags. And this head tags are unknown before executing setI18nParams as well as the switchLocalePath. So, imo, they are just as predictable. But, again, I know little to nothing about how prerender or i18n works. I can update the example with the head tags for you to have a base for testing. I'll update it tomorrow.

And yes, I saw the notice but, sincerely, I didn't understand it. "these links are updated on the client side, so this should not cause any issues." might not be accurate :smile:

Well, <client-only> just places an empty div there, so the crawler doesn't get wrong urls. That being said, that urls need to be somewhere else in the whole web to be crawled and rendered to static, which is our case.

Thank you so much for the clarification and this Nuxt module. Also thanks to @kazupon

Cheers!

Edit: also the hint about moving NuxtPage above LanguageSwitcher makes it work. So I think there has to be something else playing tricks on me/us.

BobbieGoede commented 9 months ago

Edit: also the hint about moving NuxtPage above LanguageSwitcher makes it work. So I think there has to be something else playing tricks on me.

You're calling setI18nParams in the page, by moving it up this happens before LanguageSwitcher is rendered, that's why this works.

And yes, I saw the notice but, sincerely, I didn't understand it. "these links are updated on the client side, so this should not cause any issues." might not be accurate 😄

Yes this is clearly an issue when prerendering, for SSR apps that do have client side hydration this is less of an issue as the links are updated before interaction can occur. Rendering it as expected on the server side would still be preferable.

And this head tags are unknown before executing setI18nParams as well as the switchLocalePath. So, imo, they are just as predictable. But, again, I know little to nothing about how prerender or i18n works.

The head tags are more predictable in the sense that they are handled by unhead, they can be added automatically (no templating) or by using SEO components, there's also no styling or nesting and such. Because of this it's possible to add identifier attributes to the head tags during SSR allowing them to be retrieved and replaced with up to date values.

Language switchers could be anywhere on the page (could also be more than one), using any type of element, and (generally) use the returned string of switchLocalePath. It's possible to do something similar to what unhead does, it would probably involve adding some stricter/predictable way of making a language switcher.

BobbieGoede commented 8 months ago

With #2838 merged this should be resolved when using the latest edge version, I've updated your reproduction to demonstrate here. Let me know if you're running into any issues!