fix: Handle changing href for <Link /> correctly when using `locale…

…Prefix: 'never'` (#926) Fixes #918
amannn · Mar 8, 2024 · b609dc0 · b609dc0
1 parent 0df6e42
commit b609dc0
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 21 deletions.
diff --git a/packages/next-intl/src/navigation/shared/BaseLink.tsx b/packages/next-intl/src/navigation/shared/BaseLink.tsx
@@ -27,8 +27,8 @@ function BaseLink(
   // `useParams` can be called, but the return type is `null`.
   const pathname = usePathname() as ReturnType<typeof usePathname> | null;
 
-  const defaultLocale = useLocale();
-  const isChangingLocale = locale !== defaultLocale;
+  const curLocale = useLocale();
+  const isChangingLocale = locale !== curLocale;
 
   const [localizedHref, setLocalizedHref] = useState<typeof href>(() =>
     isLocalHref(href) && (localePrefix !== 'never' || isChangingLocale)
@@ -47,17 +47,15 @@ function BaseLink(
   );
 
   function onLinkClick(event: MouseEvent<HTMLAnchorElement>) {
-    syncLocaleCookie(pathname, defaultLocale, locale);
+    syncLocaleCookie(pathname, curLocale, locale);
     if (onClick) onClick(event);
   }
 
   useEffect(() => {
-    if (!pathname || localePrefix === 'never') return;
+    if (!pathname) return;
 
-    setLocalizedHref(
-      localizeHref(href, locale, defaultLocale, pathname ?? undefined)
-    );
-  }, [defaultLocale, href, locale, localePrefix, pathname]);
+    setLocalizedHref(localizeHref(href, locale, curLocale, pathname));
+  }, [curLocale, href, locale, pathname]);
 
   if (isChangingLocale) {
     if (prefetch && process.env.NODE_ENV !== 'production') {

diff --git a/packages/next-intl/test/navigation/react-client/ClientLink.test.tsx b/packages/next-intl/test/navigation/react-client/ClientLink.test.tsx
@@ -7,6 +7,14 @@ import ClientLink from '../../../src/navigation/react-client/ClientLink';
 
 vi.mock('next/navigation');
 
+function mockLocation(pathname: string, basePath = '') {
+  vi.mocked(usePathname).mockReturnValue(pathname);
+
+  delete (global.window as any).location;
+  global.window ??= Object.create(window);
+  (global.window as any).location = {pathname: basePath + pathname};
+}
+
 describe('unprefixed routing', () => {
   beforeEach(() => {
     vi.mocked(usePathname).mockImplementation(() => '/');
@@ -104,6 +112,52 @@ describe('unprefixed routing', () => {
       screen.getByRole('link', {name: 'Test'}).getAttribute('hreflang')
     ).toBe('de');
   });
+
+  it('updates the href when the query changes for localePrefix=never', () => {
+    const {rerender} = render(
+      <ClientLink href={{pathname: '/'}} localePrefix="never">
+        Test
+      </ClientLink>
+    );
+    expect(screen.getByRole('link', {name: 'Test'}).getAttribute('href')).toBe(
+      '/'
+    );
+    rerender(
+      <ClientLink
+        href={{pathname: '/', query: {foo: 'bar'}}}
+        localePrefix="never"
+      >
+        Test
+      </ClientLink>
+    );
+    expect(screen.getByRole('link', {name: 'Test'}).getAttribute('href')).toBe(
+      '/?foo=bar'
+    );
+  });
+
+  describe('base path', () => {
+    beforeEach(() => {
+      mockLocation('/', '/base/path');
+    });
+
+    it('renders an unprefixed href when staying on the same locale', () => {
+      render(<ClientLink href="/test">Test</ClientLink>);
+      expect(
+        screen.getByRole('link', {name: 'Test'}).getAttribute('href')
+      ).toBe('/test');
+    });
+
+    it('renders a prefixed href when switching the locale', () => {
+      render(
+        <ClientLink href="/test" locale="de">
+          Test
+        </ClientLink>
+      );
+      expect(
+        screen.getByRole('link', {name: 'Test'}).getAttribute('href')
+      ).toBe('/de/test');
+    });
+  });
 });
 
 describe('prefixed routing', () => {
@@ -171,6 +225,30 @@ describe('prefixed routing', () => {
       'https://example.com/test'
     );
   });
+
+  describe('base path', () => {
+    beforeEach(() => {
+      mockLocation('/en', '/base/path');
+    });
+
+    it('renders an unprefixed href when staying on the same locale', () => {
+      render(<ClientLink href="/test">Test</ClientLink>);
+      expect(
+        screen.getByRole('link', {name: 'Test'}).getAttribute('href')
+      ).toBe('/en/test');
+    });
+
+    it('renders a prefixed href when switching the locale', () => {
+      render(
+        <ClientLink href="/test" locale="de">
+          Test
+        </ClientLink>
+      );
+      expect(
+        screen.getByRole('link', {name: 'Test'}).getAttribute('href')
+      ).toBe('/de/test');
+    });
+  });
 });
 
 describe('usage outside of Next.js', () => {
@@ -196,17 +274,24 @@ describe('usage outside of Next.js', () => {
   });
 });
 
-it('keeps the cookie value in sync', () => {
-  vi.mocked(usePathname).mockImplementation(() => '/en');
-  vi.mocked(useParams).mockImplementation(() => ({locale: 'en'}));
-  document.cookie = 'NEXT_LOCALE=en';
-
-  render(
-    <ClientLink href="/" locale="de">
-      Test
-    </ClientLink>
-  );
-  expect(document.cookie).toContain('NEXT_LOCALE=en');
-  fireEvent.click(screen.getByRole('link', {name: 'Test'}));
-  expect(document.cookie).toContain('NEXT_LOCALE=de');
+describe('cookie sync', () => {
+  beforeEach(() => {
+    vi.mocked(usePathname).mockImplementation(() => '/en');
+    vi.mocked(useParams).mockImplementation(() => ({locale: 'en'}));
+
+    mockLocation('/');
+
+    global.document.cookie = 'NEXT_LOCALE=en';
+  });
+
+  it('keeps the cookie value in sync', () => {
+    render(
+      <ClientLink href="/" locale="de">
+        Test
+      </ClientLink>
+    );
+    expect(document.cookie).toContain('NEXT_LOCALE=en');
+    fireEvent.click(screen.getByRole('link', {name: 'Test'}));
+    expect(document.cookie).toContain('NEXT_LOCALE=de');
+  });
 });