Fix search page history stack

frontends/jest-shared-setup.ts

@@ -4,6 +4,7 @@ import "cross-fetch/polyfill"
 import { configure } from "@testing-library/react"
 import { resetAllWhenMocks } from "jest-when"
 import * as matchers from "jest-extended"
+import { mockRouter } from "ol-test-utilities/mocks/nextNavigation"
 
 expect.extend(matchers)
 
@@ -92,4 +93,6 @@ afterEach(() => {
    */
   jest.clearAllMocks()
   resetAllWhenMocks()
+  mockRouter.setCurrentUrl("/")
+  window.history.replaceState({}, "", "/")
 })

frontends/main/package.json

@@ -12,7 +12,7 @@
   "dependencies": {
     "@ebay/nice-modal-react": "^1.2.13",
     "@emotion/cache": "^11.13.1",
-    "@mitodl/course-search-utils": "^3.2.3",
+    "@mitodl/course-search-utils": "3.2.4",
     "@remixicon/react": "^4.2.0",
     "@tanstack/react-query": "^4.36.1",
     "api": "workspace:*",

frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts

@@ -36,13 +36,13 @@ describe("Mock Navigation", () => {
   test("useSearchParams returns the current search params", () => {
     mockRouter.setCurrentUrl("/dynamic/bar?a=1&b=2")
     const { result } = renderHook(() => useSearchParams())
-    expect(result.current.toString()).toEqual("a=1&b=2&id=bar")
+    expect(result.current.toString()).toEqual("a=1&b=2")
   })
 
   test("useSearchParams repeats duplicate keys on the querystring", () => {
     mockRouter.setCurrentUrl("/dynamic/bar?a=1&b=2&b=3")
     const { result } = renderHook(() => useSearchParams())
-    expect(result.current.toString()).toEqual("a=1&b=2&b=3&id=bar")
+    expect(result.current.toString()).toEqual("a=1&b=2&b=3")
   })
 
   test("useParams returns the current params", () => {
@@ -58,13 +58,17 @@ describe("Mock Navigation", () => {
     const { result } = renderHook(() => nextNavigationMocks.useRouter())
     act(() => {
       result.current.push(
-        "/dynamic/foo",
+        "/dynamic/foo?c=3",
         // @ts-expect-error The type signature of mockRouter.push is for old pages router.
         // The 2nd arg here is for what our application uses, the app router
         { scroll: false },
       )
     })
-    expect(mockRouter.asPath).toBe("/dynamic/foo")
+    expect(mockRouter.asPath).toBe("/dynamic/foo?c=3")
+    act(() => {
+      result.current.push("?d=4")
+    })
+    expect(mockRouter.asPath).toBe("/dynamic/foo?d=4")
   })
 
   test("router.replace", () => {
@@ -73,12 +77,38 @@ describe("Mock Navigation", () => {
     const { result } = renderHook(() => nextNavigationMocks.useRouter())
     act(() => {
       result.current.replace(
-        "/dynamic/foo",
+        "/dynamic/foo?c=3",
         // @ts-expect-error The type signature of mockRouter.replace is for old pages router.
         // The 2nd arg here is for what our application uses, the app router
         { scroll: false },
       )
     })
-    expect(mockRouter.asPath).toBe("/dynamic/foo")
+    expect(mockRouter.asPath).toBe("/dynamic/foo?c=3")
+    act(() => {
+      result.current.push("?d=4")
+    })
+    expect(mockRouter.asPath).toBe("/dynamic/foo?d=4")
+  })
+
+  test("useSearchParams reacts to history.pushState", () => {
+    const { result } = renderHook(() => nextNavigationMocks.useSearchParams())
+    expect(result.current.toString()).toBe("")
+    const push = jest.spyOn(mockRouter, "push")
+    act(() => {
+      window.history.pushState({}, "", "/dynamic/foo?a=1&b=2")
+    })
+    expect(push).toHaveBeenCalled()
+    expect(result.current.toString()).toBe("a=1&b=2")
+  })
+
+  test("useSearchParams reacts to history.replaceState", () => {
+    const { result } = renderHook(() => nextNavigationMocks.useSearchParams())
+    expect(result.current.toString()).toBe("")
+    const replace = jest.spyOn(mockRouter, "replace")
+    act(() => {
+      window.history.replaceState({}, "", "/dynamic/foo?a=1&b=2")
+    })
+    expect(replace).toHaveBeenCalled()
+    expect(result.current.toString()).toBe("a=1&b=2")
   })
 })

frontends/ol-test-utilities/src/mocks/nextNavigation.ts

@@ -7,7 +7,6 @@
  * See https://github.com/scottrippey/next-router-mock/issues
  */
 import * as mocks from "next-router-mock"
-import { ParsedUrlQuery } from "querystring"
 import { createDynamicRouteParser } from "next-router-mock/dynamic-routes"
 
 const getParams = (template: string, pathname: string) => {
@@ -23,20 +22,6 @@ const getParams = (template: string, pathname: string) => {
   }, {})
 }
 
-/* Converts router.query objects with multiple key arrays
- * e.g. { topic: [ 'Physics', 'Chemistry' ] }
- * to [ [ 'topic', 'Physics' ], [ 'topic', 'Chemistry' ] ]
- * so that new URLSearchParams(value).toString()
- * produces topic=Physics&topic=Chemistry
- * and not topic=Physics%2CChemistry
- */
-const convertObjectToUrlParams = (obj: ParsedUrlQuery): [string, string][] =>
-  Object.entries(obj).flatMap(([key, value]) =>
-    Array.isArray(value)
-      ? value.map((v) => [key, v] as [string, string])
-      : [[key, value] as [string, string]],
-  )
-
 /**
  * memoryRouter is a mock for the older pages router
  * this file adapts it for the app router
@@ -49,8 +34,21 @@ const convertObjectToUrlParams = (obj: ParsedUrlQuery): [string, string][] =>
  */
 const originalPush = mocks.memoryRouter.push
 const originalReplace = mocks.memoryRouter.replace
-mocks.memoryRouter.push = (url) => originalPush(url)
-mocks.memoryRouter.replace = (url) => originalReplace(url)
+
+/**
+ * next-mock-router is designed for Pages router; we are adapting for App router.
+ * App router does not change pathname when pushing / replacing an href that
+ * starts with `?`.
+ */
+const prependPathIfNeeded = (url: mocks.Url) => {
+  if (typeof url === "string") {
+    const current = new URL(mockRouter.asPath, "http://localhost")
+    return url.startsWith("?") ? `${current.pathname}${url}` : url
+  }
+  return url // App router only supports strings anyway
+}
+mocks.memoryRouter.push = (url) => originalPush(prependPathIfNeeded(url))
+mocks.memoryRouter.replace = (url) => originalReplace(prependPathIfNeeded(url))
 
 export const nextNavigationMocks = {
   ...mocks,
@@ -74,10 +72,8 @@ export const nextNavigationMocks = {
   },
   useSearchParams: () => {
     const router = nextNavigationMocks.useRouter()
-
-    const search = new URLSearchParams(convertObjectToUrlParams(router.query))
-
-    return search
+    const url = new URL(router.asPath, "http://localhost")
+    return url.searchParams
   },
   useParams: () => {
     const router = nextNavigationMocks.useRouter()
@@ -86,5 +82,46 @@ export const nextNavigationMocks = {
   },
 }
 
+/**
+ * next-router-mock is built for the old NextJS Pages router, which included
+ * a { shallow: true } option on push/replace to force fully client-side routing.
+ *
+ * We're adapting next-router-mock for the NextJS App router, which removed
+ * the shallow option in favor of direct usage of window.history.pushState
+ * and window.history.replaceState for client-side routing.
+ *
+ * We patch history.pushState and history.replaceState to update the mock router
+ *
+ * Note: This is similar to what NextJS actually does for the App router.
+ * See https://github.com/vercel/next.js/blob/a52dcd54b0e419690dfedde53d09c66d71487c06/packages/next/src/client/components/app-router.tsx#L455
+ */
+const patchHistoryPushReplace = () => {
+  const originalPushState = window.history.pushState.bind(window.history)
+  window.history.pushState = (data, _unused: never, url) => {
+    originalPushState(data, "", url)
+    if (url === undefined || url === null) return
+    nextNavigationMocks.memoryRouter.push(url)
+  }
+  const originalReplaceState = window.history.replaceState.bind(window.history)
+  window.history.replaceState = (data, _unused: never, url) => {
+    originalReplaceState(data, "", url)
+    if (url === undefined || url === null) return
+    nextNavigationMocks.memoryRouter.replace(url)
+  }
+}
+
+const originalSetCurrentUrl = mocks.memoryRouter.setCurrentUrl
+mocks.memoryRouter.setCurrentUrl = (url) => {
+  // This sets it on the memoryRouter
+  originalSetCurrentUrl(url)
+  // Below we set it on window.location
+  const urlObject =
+    typeof url === "string" ? new URL(url, "http://localhost") : url
+  const pathName = urlObject.pathname ?? ""
+  const search = urlObject.search ?? ""
+  window.history.replaceState({}, "", pathName + search)
+}
+
+patchHistoryPushReplace()
 const mockRouter = nextNavigationMocks.memoryRouter
 export { mockRouter, createDynamicRouteParser }