Enhanced Link component to handle language switching and persistance properly (#36)

AlemTuzlak · web-flow · commit 6fdf8a9359b2 · 2025-04-09T12:33:16.000+02:00
# Description

The `Link` and `LanguageSwitcher` components didn't fully handle the
redirects with the `?lng=X` param in the search param, this PR aims to
address this issue and enhance the components

## Type of change

Please mark relevant options with an `x` in the brackets.

- [x] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update
- [ ] Algorithm update - updates algorithm
documentation/questions/answers etc.
- [ ] Other (please describe):

# How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide
instructions so we can reproduce. Please also
list any relevant details for your test configuration

- [ ] Integration tests
- [ ] Unit tests
- [ ] Manual tests
- [ ] No tests required

# Reviewer checklist

Mark everything that needs to be checked before merging the PR.

- [ ] Check if the UI is working as expected and is satisfactory
- [ ] Check if the code is well documented
- [ ] Check if the behavior is what is expected
- [ ] Check if the code is well tested
- [ ] Check if the code is readable and well formatted
- [ ] Additional checks (document below if any)

# Screenshots (if appropriate):

# Questions (if appropriate):
diff --git a/app/library/language-switcher/LanguageSwitcher.tsx b/app/library/language-switcher/LanguageSwitcher.tsx
@@ -6,15 +6,18 @@ import { Link } from "../link"
 const LanguageSwitcher = () => {
 	const { i18n } = useTranslation()
 	const location = useLocation()
-	const to = location.pathname
 
 	return (
 		<div className="flex gap-2 p-2 fixed top-0 right-0 w-min z-10">
 			{supportedLanguages.map((language) => (
 				<Link
 					className="text-blue-500 dark:text-white hover:underline transition-all"
 					key={language}
-					to={`${to}?lng=${language}`}
+					to={`${location.pathname}`}
+					// We override the default appending of the language to the search params via our language
+					language={language}
+					// We keep the search params if any on language change
+					keepSearchParams
 					onClick={() => i18n.changeLanguage(language)}
 				>
 					{language}
diff --git a/app/library/link/Link.browser.test.tsx b/app/library/link/Link.browser.test.tsx
@@ -0,0 +1,164 @@
+import { waitFor } from "@testing-library/react"
+import { userEvent } from "@vitest/browser/context"
+import { useLocation } from "react-router"
+import type { StubRouteEntry } from "tests/setup.browser"
+import { Link, type LinkProps } from "./link"
+const getEntries: (linkProps?: LinkProps) => StubRouteEntry[] = (linkProps) => [
+	{
+		path: "/first",
+		Component: () => {
+			const url = useLocation()
+			return (
+				<>
+					<p>
+						{url.pathname} + {url.search}
+					</p>
+					<Link {...linkProps} to="/second">
+						go
+					</Link>
+				</>
+			)
+		},
+	},
+	{
+		path: "/second",
+		Component: () => {
+			const url = useLocation()
+			return (
+				<>
+					<p>
+						{url.pathname}
+						{url.search}
+					</p>
+					<Link to="/first">go</Link>
+				</>
+			)
+		},
+	},
+]
+describe("Link", () => {
+	it("if the url is /first and you redirect to  /second nothing is added to the url", async ({ renderStub }) => {
+		const { getByText } = await renderStub({
+			entries: getEntries(),
+			props: {
+				initialEntries: ["/first"],
+			},
+		})
+		const link = getByText("go")
+		await userEvent.click(link)
+		const url = getByText("/second")
+		expect(url).toBeDefined()
+		await waitFor(() => {
+			expect(url.element()).toBeDefined()
+			expect(url.element()).toHaveTextContent("/second")
+		})
+	})
+
+	it("if the url is /first?a=1 and you redirect to /second without keepSearchParams nothing is added to the url", async ({
+		renderStub,
+	}) => {
+		const { getByText } = await renderStub({
+			entries: getEntries(),
+			props: {
+				initialEntries: ["/first?a=1"],
+			},
+		})
+		const link = getByText("go")
+		await userEvent.click(link)
+		const url = getByText("/second")
+		await waitFor(() => {
+			expect(url.element()).toBeDefined()
+			expect(url.element()).toHaveTextContent("/second")
+		})
+	})
+
+	it("if the url is /first?a=1 and you redirect to /second with keepSearchParams search params are kept", async ({
+		renderStub,
+	}) => {
+		const { getByText } = await renderStub({
+			entries: getEntries({ keepSearchParams: true, to: "/second" }),
+			props: {
+				initialEntries: ["/first?a=1"],
+			},
+		})
+		const link = getByText("go")
+		await userEvent.click(link)
+		const url = getByText("/second")
+		await waitFor(() => {
+			expect(url.element()).toBeDefined()
+			expect(url.element()).toHaveTextContent("/second?a=1")
+		})
+	})
+
+	it("if the url is /first?a=1&lng=en and you redirect to /second with keepSearchParams search params and language are kept", async ({
+		renderStub,
+	}) => {
+		const { getByText } = await renderStub({
+			entries: getEntries({ keepSearchParams: true, to: "/second" }),
+			props: {
+				initialEntries: ["/first?a=1&lng=en"],
+			},
+		})
+		const link = getByText("go")
+		await userEvent.click(link)
+		const url = getByText("/second")
+		await waitFor(() => {
+			expect(url.element()).toBeDefined()
+			expect(url.element()).toHaveTextContent("/second?a=1&lng=en")
+		})
+	})
+
+	it("if the url is /first?a=1&lng=en and you redirect to /second without keepSearchParams language is kept", async ({
+		renderStub,
+	}) => {
+		const { getByText } = await renderStub({
+			entries: getEntries({ to: "/second" }),
+			props: {
+				initialEntries: ["/first?lng=en"],
+			},
+		})
+		const link = getByText("go")
+		await userEvent.click(link)
+		const url = getByText("/second")
+		await waitFor(() => {
+			expect(url.element()).toBeDefined()
+			expect(url.element()).toHaveTextContent("/second?lng=en")
+		})
+	})
+
+	it("if the url is /first?a=1&lng=en and you redirect to /second with a language override it is changed and search params are removed", async ({
+		renderStub,
+	}) => {
+		const { getByText } = await renderStub({
+			entries: getEntries({ to: "/second", language: "bs" }),
+			props: {
+				initialEntries: ["/first?lng=en"],
+			},
+		})
+		const link = getByText("go")
+		await userEvent.click(link)
+		const url = getByText("/second")
+		await waitFor(() => {
+			expect(url.element()).toBeDefined()
+			expect(url.element()).toHaveTextContent("/second?lng=bs")
+		})
+	})
+
+	it("if the url is /first?a=1&lng=en and you redirect to /second with a language override it is changed and search params are kept with keepSearchParams", async ({
+		renderStub,
+	}) => {
+		const { getByText } = await renderStub({
+			entries: getEntries({ to: "/second", language: "bs", keepSearchParams: true }),
+			props: {
+				initialEntries: ["/first?a=a&lng=en"],
+			},
+		})
+		const link = getByText("go")
+		await userEvent.click(link)
+		const url = getByText("/second")
+		await waitFor(() => {
+			expect(url.element()).toBeDefined()
+			expect(url.element()).toHaveTextContent("/second?a=a&lng=bs")
+		})
+	})
+})
diff --git a/app/library/link/link.tsx b/app/library/link/link.tsx
@@ -1,7 +1,20 @@
 import { Link as ReactRouterLink, type LinkProps as ReactRouterLinkProps } from "react-router"
+import type { Language } from "~/localization/resource"
+import { useEnhancedTo } from "./useEnhancedTo"
 
-interface LinkProps extends ReactRouterLinkProps {}
+export interface LinkProps extends ReactRouterLinkProps {
+	keepSearchParams?: boolean
+	language?: Language
+}
 
-export const Link = ({ prefetch = "intent", viewTransition = true, ...props }: LinkProps) => {
-	return <ReactRouterLink prefetch={prefetch} viewTransition={viewTransition} {...props} />
+export const Link = ({
+	prefetch = "intent",
+	viewTransition = true,
+	keepSearchParams = false,
+	to,
+	language,
+	...props
+}: LinkProps) => {
+	const enhancedTo = useEnhancedTo({ language, to, keepSearchParams })
+	return <ReactRouterLink prefetch={prefetch} viewTransition={viewTransition} to={enhancedTo} {...props} />
 }
diff --git a/app/library/link/useEnhancedTo.ts b/app/library/link/useEnhancedTo.ts
@@ -0,0 +1,52 @@
+import { useMemo } from "react"
+import { type To, useSearchParams } from "react-router"
+import type { Language } from "~/localization/resource"
+
+/**
+ * Enhances the default to prop by adding the language to the search params and conditionally keeping the search params
+ * @param language The language to use over the search param language
+ * @param to The new location to navigate to
+ * @param keepSearchParams Whether to keep the search params or not
+ *
+ * @example
+ * ```tsx
+ * // override the language
+ * function Component(){
+ * 	const enhancedTo = useEnhancedTo({ language: "en", to: "/" })
+ * 	return <Link to={enhancedTo} /> // Will navigate to /?lng=en even if the current url contains a different lanugage
+ * }
+ *
+ * function Component(){
+ * 	const enhancedTo = useEnhancedTo({ to: "/" })
+ * 	return <Link to={enhancedTo} /> // Will navigate to /?lng=X where X is the current language in the url search params, or just to / if no language is found
+ * }
+ *
+ * function Component(){
+ * 	const enhancedTo = useEnhancedTo({ to: "/", keepSearchParams: true })
+ * 	return <Link to={enhancedTo} /> // Will navigate to /?params=from_the_url_search_params&lng=en
+ * }
+ * ```
+ */
+export const useEnhancedTo = ({
+	language,
+	to,
+	keepSearchParams,
+}: { language?: Language; to: To; keepSearchParams?: boolean }) => {
+	const [params] = useSearchParams()
+	const { lng, ...searchParams } = Object.fromEntries(params.entries())
+	// allow language override for language switcher or manually setting the language in specific cases
+	const lang = language ?? params.get("lng")
+	const newSearchParams = new URLSearchParams(searchParams)
+	const searchString = newSearchParams.toString()
+	const hasSearchParams = searchString.length > 0
+	const appendSearchParams = lang || hasSearchParams
+	const newPath = useMemo(
+		() =>
+			to +
+			(appendSearchParams
+				? `?${keepSearchParams && hasSearchParams ? `${searchString}${lang ? "&" : ""}` : ""}${lang ? `lng=${lang}` : ""}`
+				: ""),
+		[to, appendSearchParams, keepSearchParams, hasSearchParams, searchString, lang]
+	)
+	return newPath
+}
diff --git a/tests/setup.browser.tsx b/tests/setup.browser.tsx
@@ -6,7 +6,7 @@ import { Outlet, type RoutesTestStubProps, createRoutesStub } from "react-router
 import { render } from "vitest-browser-react"
 import i18n from "~/localization/i18n"
 import { type Language, type Namespace, resources } from "~/localization/resource"
-type StubRouteEntry = Parameters<typeof createRoutesStub>[0][0]
+export type StubRouteEntry = Parameters<typeof createRoutesStub>[0][0]
 
 const renderStub = async (args?: {
 	props?: RoutesTestStubProps
diff --git a/vitest.workspace.ts b/vitest.workspace.ts
@@ -26,9 +26,11 @@ export default defineWorkspace([
 			include: ["./**.test.{ts,tsx}", "./**.browser.test.{ts,tsx}", "!./**.server.test.{ts,tsx}"],
 			setupFiles: ["./tests/setup.browser.tsx"],
 			name: "browser tests",
+
 			browser: {
 				enabled: true,
 				instances: [{ browser: "chromium" }],
+
 				provider: "playwright",
 				// https://playwright.dev
 				//providerOptions: {},
