-
-
Notifications
You must be signed in to change notification settings - Fork 246
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
Improve initial render of Link
for localePrefix: 'as-necessary'
and make getPathname
more useful
#444
Comments
@jlalmes Can you provide a screenshot from the Google Search Console issue that shows the issue? Maybe I was a bit too naive to think that this isn't an issue for Google, since we a) always have a working URL with the redirect and b) patch it up as necessary on the client side. |
Hmm, I see—thanks! Is maybe |
Small update: The factory functions for navigation APIs that are being worked on in #426 could help to mitigate this for That means that if the default locale can only be determined in the middleware, we can't statically render different link targets on the server side. If the navigation APIs would also accept domain config, we could check if the user doesn't use domain config to mitigate this, but this gets quite complex and still doesn't fix all cases (i.e. when the user uses a domain config). Also, we have to accept domain config only to detect if the user doesn't use one—seems a bit backward. I'm wondering if it would be a good idea to flip the default for Update: With the release candidate, the default for |
defaultLocale
and localePrefix
for initial render of Link
when localePrefix: 'as-necessary'
defaultLocale
and localePrefix
for initial render of Link
when localePrefix: 'as-necessary'
or localePrefix: 'never'
defaultLocale
and localePrefix
for initial render of Link
when localePrefix: 'as-necessary'
or localePrefix: 'never'
defaultLocale
and localePrefix
for initial render of Link
when localePrefix
is as-necessary
or never
@amannn Could this be fixed more easily for when the |
That's a good point actually. Based on your use case, in 18157bc I've added a note that With that, we could consider:
Would you be interested in working on this? |
@amannn sounds like a plan! For now I have added a wrapper component that seems to do the trick: "use client";
import {
type I18nLink,
type LocaleCode,
getPathname,
type pathnames,
} from "@/lib/navigation";
import { useLocale } from "next-intl";
import NextLink from "next/link";
import { type ComponentProps } from "react";
export function Link<Pathname extends keyof typeof pathnames>({
href,
...rest
}: ComponentProps<typeof I18nLink<Pathname>>) {
const locale = useLocale() as LocaleCode;
// @ts-expect-error
const localizedHref = getPathname({ href, locale });
return <NextLink href={localizedHref} {...rest} />;
} |
Sounds good! I've started working on an implementation. |
…render of `Link` when using `localePrefix: never`. Also fix edge case in middleware when using localized pathnames for redirects that remove a locale prefix (fixes an infinite loop). (#678) By accepting an optional `localePrefix` for the navigation APIs, we can get the initial render of the `href` of `Link` right if `localePrefix: 'never'` is set. This can be helpful if domain-based routing is used and you have a single locale per domain. Note that this change is backward-compatible. It's now recommended to set the `localePrefix` for the navigation APIs to get improved behavior for `Link` in case `localePrefix: 'never'` is used, but otherwise your app will keep working with the previous behavior. Ref #444
@zipme I've addressed your use case in #678, this should help! In regard to this issue: As a second step, if the navigation APIs would accept This would require a major version though to require |
defaultLocale
and localePrefix
for initial render of Link
when localePrefix
is as-necessary
or never
defaultLocale
and localePrefix
for initial render of Link
when localePrefix
is as-necessary
defaultLocale
and localePrefix
for initial render of Link
when localePrefix
is as-necessary
localePrefix
, defaultLocale
and domains
for navigation APIs
localePrefix
, defaultLocale
and domains
for navigation APIslocalePrefix
, defaultLocale
and domains
for navigation APIs to improve handling of localePrefix: 'as-necessary'
localePrefix
, defaultLocale
and domains
for navigation APIs to improve handling of localePrefix: 'as-necessary'
localePrefix
, defaultLocale
and domains
for navigation APIs
I was fiddling around with the navigation APIs these days, and realised that it seems impossible (at least for me) to make the |
Replied here: #653 (comment) |
localePrefix
, defaultLocale
and domains
for navigation APIsLink
for localePrefix: 'as-necessary'
and make getPathname
more useful
When using
And calling I think it's possible and not too difficult if we also pass in the export const {
redirect,
Link
} = createLocalizedPathnamesNavigation({ locales, pathnames, localePrefix, defaultLocale }); To let it redirect correctly right? But indeed, for domains configuration this would still be quite complicated I suppose |
I am doing this now as a workaround: import { type RedirectType, redirect as nextRedirect } from "next/navigation";
import { locales, pathnames, localePrefix, defaultLocale } from './config'
import NextLink from "next/link";
export const {
redirect,
getPathname: getPathname_
} = createLocalizedPathnamesNavigation({ locales, pathnames, localePrefix });
export const getPathname: typeof getPathname_ = ({ href, locale }) => {
const pathname = getPathname_({ href, locale });
let prefix = "";
if (locale !== defaultLocale) {
prefix = `/${locale}`;
}
return `${prefix}${pathname}`;
};
export const redirect: typeof redirect_ = (href, type) => {
const locale = useLocale();
const path = getPathname({ href, locale });
nextRedirect(path, type);
};
export function Link<Pathname extends keyof typeof pathnames>(
{ href, ...rest }: ComponentProps<typeof I18nLink<Pathname>>,
ref: ForwardedRef<ElementRef<typeof NextLink>>,
) {
const locale = useLocale();
// @ts-expect-error this is okay
const localizedHref = getPathname({ href, locale });
return <NextLink href={localizedHref} {...rest} ref={ref} />;
} |
Thanks for workarounds. Hope, that fix will be rolled out soon. As there is no point to initially render locale in tag, as it is causing SEO issues. |
This PR provides a new **`createNavigation`** function that supersedes the previously available APIs: 1. `createSharedPathnamesNavigation` 2. `createLocalizedPathnamesNavigation` The new function unifies the API for both use cases and also fixes a few quirks in the previous APIs. **Usage** ```tsx import {createNavigation} from 'next-intl/navigation'; import {defineRouting} from 'next-intl/routing'; export const routing = defineRouting(/* ... */); export const {Link, redirect, usePathname, useRouter} = createNavigation(routing); ``` (see the [updated navigation docs](https://next-intl-docs-git-feat-create-navigation-next-intl.vercel.app/docs/routing/navigation)) **Improvements** 1. A single API can be used both for shared as well as localized pathnames. This reduces the API surface and simplifies the corresponding docs. 2. `Link` can now be composed seamlessly into another component with its `href` prop without having to add a generic type argument. 3. `getPathname` is now available for both shared as well as localized pathnames (fixes #785) 4. `router.push` and `redirect` now accept search params consistently via the object form (e.g. `router.push({pathname: '/users', query: {sortBy: 'name'})`)—regardless of if you're using shared or localized pathnames. 5. When using `localePrefix: 'as-necessary'`, the initial render of `Link` now uses the correct pathname immediately during SSR (fixes [#444](#444)). Previously, a prefix for the default locale was added during SSR and removed during hydration. Also `redirect` now gets the final pathname right without having to add a superfluous prefix (fixes [#1335](#1335)). The only exception is when you use `localePrefix: 'as-necessary'` in combination with `domains` (see [Special case: Using `domains` with `localePrefix: 'as-needed'`](https://next-intl-docs-git-feat-create-navigation-next-intl.vercel.app/docs/routing#domains-localeprefix-asneeded)) 6. `Link` is now compatible with the `asChild` prop of Radix Primitives when rendered in RSC (see [#1322](#1322)) **Migrating to `createNavigation`** `createNavigation` is generally considered a drop-in replacement, but a few changes might be necessary: 1. `createNavigation` is expected to receive your complete routing configuration. Ideally, you define this via the [`defineRouting`](https://next-intl-docs.vercel.app/docs/routing#define-routing) function and pass the result to `createNavigation`. 2. If you've used `createLocalizedPathnamesNavigation` and have [composed the `Link` with its `href` prop](https://next-intl-docs.vercel.app/docs/routing/navigation#link-composition), you should no longer provide the generic `Pathname` type argument (see [updated docs](https://next-intl-docs-git-feat-create-navigation-next-intl.vercel.app/docs/routing/navigation#link-composition)). ```diff - ComponentProps<typeof Link<Pathname>> + ComponentProps<typeof Link> ``` 3. If you've used [`redirect`](https://next-intl-docs.vercel.app/docs/routing/navigation#redirect), you now have to provide an explicit locale (even if it's just [the current locale](https://next-intl-docs.vercel.app/docs/usage/configuration#locale)). This change was necessary for an upcoming change in Next.js 15 where `headers()` turns into a promise (see [#1375](#1375) for details). ```diff - redirect('/about') + redirect({pathname: '/about', locale: 'en'}) ``` 4. If you've used [`getPathname`](https://next-intl-docs.vercel.app/docs/routing/navigation#getpathname) and have previously manually prepended a locale prefix, you should no longer do so—`getPathname` now takes care of this depending on your routing strategy. ```diff - '/'+ locale + getPathname(/* ... */) + getPathname(/* ... */); ``` 5. If you're using a combination of `localePrefix: 'as-necessary'` and `domains` and you're using `getPathname`, you now need to provide a `domain` argument (see [Special case: Using `domains` with `localePrefix: 'as-needed'`](https://next-intl-docs-git-feat-create-navigation-next-intl.vercel.app/docs/routing#domains-localeprefix-asneeded))
A quick note here: The feedback raised in this issue has been addressed as part of an upcoming Also, thanks again for the great feedback here! 🙏❤️ |
I also encountered this problem. I temporarily created a custom Link and the test was ok.
|
Is your feature request related to a problem? Please describe.
The
localePrefix: 'as-necessary'
strategy currently has these drawbacks:Link
in RSC always includes a prefix, which we potentially remove during hydration if we detect that no locale prefix should be used.getPathname
is somewhat hard to use, because users have to specify their own logic to check if a prefix should be added.Describe the solution you'd like
localePrefix
,defaultLocale
anddomains
, all being expected to be provided in case you use them. We should move all routing config to a shared type that can be shared with the middleware. Update: Done in feat: AdddefineRouting
for easier i18n routing setup #1299Link
should consider the new options for the initial render (see Improve initial render ofLink
forlocalePrefix: 'as-necessary'
and makegetPathname
more useful #444 (comment)). IfLink
receiveslocales
, we should be able to useusePathname
to see if the current pathname is prefixed and use this information to get the initial SSR render right.getPathname
should consider the options too and return a final pathname (with/without a locale prefix, with/without domain). Furthermore,getPathname
should be supported for shared pathnames too.domain
if a user wants to link/redirect to another domain.This will require a major version to ensure users supply the new options (related to #779).
In regard to (2) there's an edge case for
localePrefix: 'as-necessary'
anddomains
. The domain is only available at request time, therefore we have to either a) support SSG and potentially patch thehref
ofLink
on the client side or b) require dynamic rendering and return a final URL. This is TBD. Note thatgetPathname
only has "one shot" to return a final URL, which would certainly require reading from headers. An idea could be to require dynamic rendering in casedomains
are configured, but to point users towards the alternative of building a separate app per domain if SSG is important.Make sure to remove the note in the
localePrefix: 'as-needed'
docs once this is implemented.Describe alternatives you've considered
The problems outlined above are mostly related to
localePrefix: 'as-necessary'
.You can avoid these by either:
localePrefix: 'always'
localePrefix: 'never'
and supplying this to the navigation factory function (already supported)The text was updated successfully, but these errors were encountered: