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: createNavigation #1316

Merged
merged 68 commits into from
Oct 1, 2024
Merged

feat: createNavigation #1316

merged 68 commits into from
Oct 1, 2024

Conversation

amannn
Copy link
Owner

@amannn amannn commented Sep 3, 2024

This PR provides a new createNavigation function that supersedes the previously available APIs:

  1. createSharedPathnamesNavigation
  2. createLocalizedPathnamesNavigation

This new function is a reimplementation of the existing navigation functionality that unifies the API for both use cases and also fixes a few quirks in the previous APIs.

Usage

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)

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 Managing canonical links #785)
  4. For improved compatibility, 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. You can still use router.push('/users?sortBy=name') if you prefer though.
  5. When using localePrefix: 'as-needed', the initial render of Link now uses the correct pathname immediately during SSR (fixes #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). The only exception is when you use localePrefix: 'as-needed' in combination with domains (see Special case: Using domains with localePrefix: 'as-needed')
  6. Slightly smaller in size: createNavigation weighs 2.98 kB (createSharedPathnamesNavigation was 3.01 kB, createLocalizedPathnamesNavigation was 3.27 kB)
  7. Prepares next-intl for Next.js 15 to run without warnings.

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 function and pass the result to createNavigation.
  2. If you've used createLocalizedPathnamesNavigation and have composed the Link with its href prop, you should no longer provide the generic Pathname type argument (see updated docs).
- ComponentProps<typeof Link<Pathname>>
+ ComponentProps<typeof Link>
  1. If you've used redirect, you now have to provide an explicit locale (even if it's just the current locale). The previously passed href (whether it was a string or an object) now needs to be wrapped in an object and assigned to the href prop. This change was necessary for an upcoming change in Next.js 15 where headers() turns into a promise (see #1375 for details).
// Retrieving the current locale
// ... in regular components:
const locale = useLocale();
// ... in async components:
const locale = await getLocale();
- redirect('/about')
+ redirect({href: '/about', locale})

- redirect({pathname: '/users/[id]', params: {id: 2}})
+ redirect({href: {pathname: '/users/[id]', params: {id: 2}}, locale})
  1. If you've used 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.
- '/'+ locale + getPathname(/* ... */)
+ getPathname(/* ... */);
  1. If you're using a combination of localePrefix: 'as-needed' and domains and you're using getPathname, you now need to provide a domain argument (see Special case: Using domains with localePrefix: 'as-needed')

Copy link

vercel bot commented Sep 3, 2024

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

Name Status Preview Comments Updated (UTC)
next-intl-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 1, 2024 3:25pm
next-intl-example-app-router ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 1, 2024 3:25pm
next-intl-example-app-router-without-i18n-routing ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 1, 2024 3:25pm

docs/pages/docs/routing.mdx Outdated Show resolved Hide resolved
docs/pages/docs/routing.mdx Outdated Show resolved Hide resolved
@DevOsaWebsite
Copy link

DevOsaWebsite commented Oct 7, 2024

@amannn

@DevOsaWebsite Thanks for providing feedback here!

Some questions:

Besides updating the version, did you upgrade to createNavigation?
Are you referring to the markup resulting from the server side render, or the final markup after hydration that renders in the browser?
Are you using domains in combination with localePrefix: 'as-needed'?
Please refer to #1316, you should find an answer there.

  1. Yes. You can see it here -- stackoverflow

  2. About SSR (I am primarily a technical SEO + marketing specialist), SSR and avoidance of third-party side (use)Effects are important for me. Therefore, before writing a ticket, I checked with the eyes of a Google bot: in the Google browser console with the disabled JS, through View Page Source, and through the HTML validator. In all cases, there was a defaultLocale prefix in the DOM, which will create a lot of problems for multi-page sites.

Screenshot 2024-10-07 at 19 33 38

As far as I understand, the prefix is ​​important for the feature related to locale detection for "x-middleware-rewrite". Then I'm
then I tried to turn it off

const handleI18nRouting = createMiddleware(routing, { localeDetection: false, });
but the problem is still there

then i went into the code and found the function

ClientLink.js

const finalLocale = locale || defaultLocale;
const prefix = utils.getLocalePrefix(finalLocale, localePrefix);

utils.js

function getLocalePrefix(locale, localePrefix) {
  var _localePrefix$prefixe;
  return localePrefix.mode !== 'never' && ((_localePrefix$prefixe = localePrefix.prefixes) === null || _localePrefix$prefixe === void 0 ? void 0 : _localePrefix$prefixe[locale]) ||
  // We return a prefix even if `mode: 'never'`. It's up to the consumer
  // to decide to use it or not.
  '/' + locale;
}

locale || defaultLocale - no chance 😀😀😀

  1. not use domains, but use as-needed
import {DEFAULT_LOCALE, LOCALES} from '@/config/locale';
import {type DomainsConfig} from 'next-intl/routing';

const locales = LOCALES as never as string[];
export const config: {
  locales: string[];
  defaultLocale: string;
  localePrefix: 'always' | 'as-needed' | 'never';
  domains?: DomainsConfig<string[]>;
} = {
  locales,
  defaultLocale: DEFAULT_LOCALE,
  localePrefix: 'as-needed',
};

UPD
p.s.

Before using next-intl - worked with custom i18n and encountered a similar problem, I solved it by off localeDetecting and reworked the function

 const clearPathnameFromLocale = (pathname: string): string =>
  pathname.replace(new RegExp(`^\\/(${LOCALES.join('|')})`), '');

const isIncludeLocale = (path: string): boolean =>
  new RegExp(
    `^\\/(${LOCALES.filter((locale) => locale !== DEFAULT_LOCALE).join('|')})(/|$)`,
  ).test(path);

const isExternalPath = (path: string): boolean =>
  ['http', 'https', 'tel:', 'mailto:', 'data:'].some((prefix) =>
    path.includes(prefix),
  );

const localizedPath = (
  path: string,
  locale: string = DEFAULT_LOCALE,
  options?: {replaceLocaleIfDifferent?: boolean},
): string => {
  const {replaceLocaleIfDifferent} = options || {};

  if (
    isExternalPath(path) ||
    (!replaceLocaleIfDifferent &&
      (locale === DEFAULT_LOCALE || isIncludeLocale(path)))
  ) {
    return path;
  }

  if (replaceLocaleIfDifferent) {
    return locale === DEFAULT_LOCALE
      ? clearPathnameFromLocale(path)
      : `/${locale}${clearPathnameFromLocale(path)}`;
  }

  return `/${locale}${path.startsWith('/') ? path : `/${path}`}`;
};

some tests on the topic

it('should replace the locale if it is different and replaceLocaleIfDifferent is true', () => {
    const path = '/pl/about';
    const locale = 'en';
    const expected = '/about';

    const result = localizedPath(path, locale, {
      replaceLocaleIfDifferent: true,
    });

    expect(result).toBe(expected);
  });

  it('should not replace the locale if it is different and replaceLocaleIfDifferent is false', () => {
    const path = '/pl/about';
    const locale = 'en';
    const expected = '/pl/about';

    const result = localizedPath(path, locale, {
      replaceLocaleIfDifferent: false,
    });

    expect(result).toBe(expected);
  });

to catch 404, added

src/app/[locale]/[...rest]/page.tsx

const RestPage = (): React.JSX.Element => {
  const userAgent = headers().get('user-agent');
  const xPathname = headers().get('x-pathname');

  //TODO - add api service to log 404 pages
  Logger.error(`404 Page not found: ${xPathname} / ${userAgent}`);

  notFound();
};

@amannn
Copy link
Owner Author

amannn commented Oct 8, 2024

@DevOsaWebsite

Thanks for including a link to your Stack Overflow question!

The reason why a prefix is rendered, is because you're linking to the default locale from a locale switcher (i.e. you're providing the locale prop). In this case, it's expected to link to /en, because if the user is on a secondary locale (that is prefixed) and would navigate to /, the cookie would redirect the user back to the secondary locale.

Instead, in this case the user should navigate to /en explicitly, update the cookie value and then go back to /.

Here's a test for the relevant implementation:

it('renders a prefix when currently on a secondary locale and linking to the default locale', () => {
mockCurrentLocale('de');
const markup = renderToString(
<Link href="/about" locale="en">
About
</Link>
);
expect(markup).toContain('href="/en/about"');
});

If you want to avoid this, you could for example implement a locale switcher based on a select and use useRouter().push instead (the App Router with i18n routing example does that).

Does this help? I should probably mention this in the docs …

@amannn amannn deleted the feat/create-navigation branch October 24, 2024 13:30
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.

2 participants