fix: Handle external URLs passed to redirect and permanentRedirect

[mhogeveen]

This PR addresses an issue where absolute URL's used as pathnames in the redirect and permanentRedirect functions returned from createSharedPathnamesNavigation are localized.

Current behaviour:

const { redirect, permanentRedirect } =
  createSharedPathnamesNavigation({
    locales: ['en-GB'],
    localePrefix: 'as-needed',
  })

redirect('https://example.com')
// will result in a redirect to '/en-GBhttps://example.com'

permanentRedirect('https://example.com')
// will result in a redirect to '/en-GBhttps://example.com'

Expected behaviour:

const { redirect, permanentRedirect } =
  createSharedPathnamesNavigation({
    locales: ['en-GB'],
    localePrefix: 'as-needed',
  })

redirect('https://example.com')
// will result in a redirect to 'https://example.com'

permanentRedirect('https://example.com')
// will result in a redirect to 'https://example.com'

Proposed solution:

Include a check whether to localize the given pathname based on the path starting with a / in:

• packages/next-intl/src/navigation/shared/baseRedirect.tsx
• packages/next-intl/src/navigation/shared/basePermanentRedirect.tsx

If there are any questions, feedback, and or further improvements required please let me know 😄

[vercel]

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	May 23, 2024 6:38am
next-intl-example-app-router	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 23, 2024 6:38am

[vercel]

@mhogeveen is attempting to deploy a commit to the next-intl Team on Vercel.

A member of the Team first needs to authorize it.

[amannn]

That sounds reasonable, thanks for the proposal!

Just some minor remarks:

1. We had a similar PR for Link: feat: Improvements for localized Link #257. Can we make sure that we share the detection wether a link is external or not?
2. Could you add some tests for the new behavior?
3. If you don't mind, could you update useRouter to use this logic too?

Thanks a lot!

[mhogeveen]

I've just updated the PR to use the same external link detection used in the ClientLink component and I've added a couple of tests for the baseRedirect and basePermanentRedirect functions. That covers point 1 & 2. Point 3 however already seems to be the case:

next-intl/packages/next-intl/src/navigation/react-client/useBaseRouter.tsx

Line 44 in 312956e

return localizeHref(href, nextLocale || locale, locale, curPathname);

next-intl/packages/next-intl/src/shared/utils.tsx

Lines 33 to 53 in 312956e

	export function localizeHref(
	href: UrlObject | string,
	locale: string,
	curLocale: string = locale,
	curPathname: string
	) {
	if (!isLocalHref(href) || isRelativeHref(href)) {
	return href;
	}
	const isSwitchingLocale = locale !== curLocale;
	const isPathnamePrefixed =
	locale == null || hasPathnamePrefixed(locale, curPathname);
	const shouldPrefix = isSwitchingLocale || isPathnamePrefixed;
	if (shouldPrefix && locale != null) {
	return prefixHref(href, locale);
	}
	return href;
	}

If I'm missing something let me know!

[amannn]

Fantastic PR, thanks a lot @mhogeveen! 🙌

I've added another commit with minor cleanup stuff, hope you don't mind.

Thanks also for checking useRouter! You're right, all the necessary handling was already in place there, only the redirect functions were missing.

[mhogeveen]

Thanks for the quick response @amannn. It's much appreciated!