From db1cebb730451e0f6aa21f7937845198e4d3f007 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 10 Oct 2024 10:20:22 -0400 Subject: [PATCH 1/4] make mocks/nextNavigation respect client nav --- frontends/jest-shared-setup.ts | 3 + .../src/mocks/nextNavigation.test.ts | 42 ++++++++-- .../src/mocks/nextNavigation.ts | 79 ++++++++++++++----- 3 files changed, 97 insertions(+), 27 deletions(-) diff --git a/frontends/jest-shared-setup.ts b/frontends/jest-shared-setup.ts index 76e8c247db..1195a262ae 100644 --- a/frontends/jest-shared-setup.ts +++ b/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({}, "", "/") }) diff --git a/frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts b/frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts index d26652f00e..04c5872ecc 100644 --- a/frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts +++ b/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") }) }) diff --git a/frontends/ol-test-utilities/src/mocks/nextNavigation.ts b/frontends/ol-test-utilities/src/mocks/nextNavigation.ts index 978e230150..c1cca3cfb7 100644 --- a/frontends/ol-test-utilities/src/mocks/nextNavigation.ts +++ b/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 } From d82ac96134da7ac5cbb6a2e446b8fc502edf3a63 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 10 Oct 2024 16:48:21 -0400 Subject: [PATCH 2/4] bump course-search-utils --- frontends/main/package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frontends/main/package.json b/frontends/main/package.json index c88280b983..4d0e745025 100644 --- a/frontends/main/package.json +++ b/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": "https://github.com/mitodl/course-search-utils.git#506ee47d6145837b8f2d3a2add9258c86b390f57", "@remixicon/react": "^4.2.0", "@tanstack/react-query": "^4.36.1", "api": "workspace:*", diff --git a/yarn.lock b/yarn.lock index d66180e61a..be9313743e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3527,9 +3527,9 @@ __metadata: languageName: node linkType: hard -"@mitodl/course-search-utils@npm:^3.2.3": +"@mitodl/course-search-utils@https://github.com/mitodl/course-search-utils.git#506ee47d6145837b8f2d3a2add9258c86b390f57": version: 3.2.3 - resolution: "@mitodl/course-search-utils@npm:3.2.3" + resolution: "@mitodl/course-search-utils@https://github.com/mitodl/course-search-utils.git#commit=506ee47d6145837b8f2d3a2add9258c86b390f57" dependencies: "@mitodl/open-api-axios": "npm:2024.9.16" "@remixicon/react": "npm:^4.2.0" @@ -3549,7 +3549,7 @@ __metadata: optional: true react-router: optional: true - checksum: 10/57484acd65d63289f9c539f687af738a079b233848b0c4b25855691491c3a15ff8f98e06d3065519aaee980f10a9d351507fd05abeb95cd667f0b9358d3d8521 + checksum: 10/1d94934efb27d99540c1428f1056017d8447f05ad2f0cc4ebb4bf13075a74371f3cbd09b1a17b5e14bf489265244015b6483c94db9ecaba001eeef228d657f91 languageName: node linkType: hard @@ -13669,7 +13669,7 @@ __metadata: "@ebay/nice-modal-react": "npm:^1.2.13" "@emotion/cache": "npm:^11.13.1" "@faker-js/faker": "npm:^8.4.1" - "@mitodl/course-search-utils": "npm:^3.2.3" + "@mitodl/course-search-utils": "https://github.com/mitodl/course-search-utils.git#506ee47d6145837b8f2d3a2add9258c86b390f57" "@remixicon/react": "npm:^4.2.0" "@tanstack/react-query": "npm:^4.36.1" "@testing-library/jest-dom": "npm:^6.4.8" From ccb8350ee83da426d27d393abbc0559cb04f8b69 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 11 Oct 2024 07:58:54 -0400 Subject: [PATCH 3/4] bump-course-search --- frontends/main/package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frontends/main/package.json b/frontends/main/package.json index 4d0e745025..1f682c5a23 100644 --- a/frontends/main/package.json +++ b/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": "https://github.com/mitodl/course-search-utils.git#506ee47d6145837b8f2d3a2add9258c86b390f57", + "@mitodl/course-search-utils": "https://github.com/mitodl/course-search-utils.git#2be637effa7e3bdaf9adcbf98a04724cba2753b1", "@remixicon/react": "^4.2.0", "@tanstack/react-query": "^4.36.1", "api": "workspace:*", diff --git a/yarn.lock b/yarn.lock index be9313743e..9982632b50 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3527,9 +3527,9 @@ __metadata: languageName: node linkType: hard -"@mitodl/course-search-utils@https://github.com/mitodl/course-search-utils.git#506ee47d6145837b8f2d3a2add9258c86b390f57": +"@mitodl/course-search-utils@https://github.com/mitodl/course-search-utils.git#2be637effa7e3bdaf9adcbf98a04724cba2753b1": version: 3.2.3 - resolution: "@mitodl/course-search-utils@https://github.com/mitodl/course-search-utils.git#commit=506ee47d6145837b8f2d3a2add9258c86b390f57" + resolution: "@mitodl/course-search-utils@https://github.com/mitodl/course-search-utils.git#commit=2be637effa7e3bdaf9adcbf98a04724cba2753b1" dependencies: "@mitodl/open-api-axios": "npm:2024.9.16" "@remixicon/react": "npm:^4.2.0" @@ -3549,7 +3549,7 @@ __metadata: optional: true react-router: optional: true - checksum: 10/1d94934efb27d99540c1428f1056017d8447f05ad2f0cc4ebb4bf13075a74371f3cbd09b1a17b5e14bf489265244015b6483c94db9ecaba001eeef228d657f91 + checksum: 10/c1e802e77d0580b5e04748386bc361779ec250f8093613c72615b53a096928e8021cfdcae2488edeb1eaabfc8114c8f8c9dd73b651ffd248432c91cb6a925e30 languageName: node linkType: hard @@ -13669,7 +13669,7 @@ __metadata: "@ebay/nice-modal-react": "npm:^1.2.13" "@emotion/cache": "npm:^11.13.1" "@faker-js/faker": "npm:^8.4.1" - "@mitodl/course-search-utils": "https://github.com/mitodl/course-search-utils.git#506ee47d6145837b8f2d3a2add9258c86b390f57" + "@mitodl/course-search-utils": "https://github.com/mitodl/course-search-utils.git#2be637effa7e3bdaf9adcbf98a04724cba2753b1" "@remixicon/react": "npm:^4.2.0" "@tanstack/react-query": "npm:^4.36.1" "@testing-library/jest-dom": "npm:^6.4.8" From ff69f165b5e188607894e59c4d8ab6f6700b7cd9 Mon Sep 17 00:00:00 2001 From: Jon Kafton <939376+jonkafton@users.noreply.github.com> Date: Tue, 15 Oct 2024 13:22:25 +0200 Subject: [PATCH 4/4] Bump to published course-search-utils --- frontends/main/package.json | 2 +- yarn.lock | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/frontends/main/package.json b/frontends/main/package.json index 1f682c5a23..063694aba1 100644 --- a/frontends/main/package.json +++ b/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": "https://github.com/mitodl/course-search-utils.git#2be637effa7e3bdaf9adcbf98a04724cba2753b1", + "@mitodl/course-search-utils": "3.2.4", "@remixicon/react": "^4.2.0", "@tanstack/react-query": "^4.36.1", "api": "workspace:*", diff --git a/yarn.lock b/yarn.lock index 9982632b50..d467ecc53b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3527,9 +3527,9 @@ __metadata: languageName: node linkType: hard -"@mitodl/course-search-utils@https://github.com/mitodl/course-search-utils.git#2be637effa7e3bdaf9adcbf98a04724cba2753b1": - version: 3.2.3 - resolution: "@mitodl/course-search-utils@https://github.com/mitodl/course-search-utils.git#commit=2be637effa7e3bdaf9adcbf98a04724cba2753b1" +"@mitodl/course-search-utils@npm:3.2.4": + version: 3.2.4 + resolution: "@mitodl/course-search-utils@npm:3.2.4" dependencies: "@mitodl/open-api-axios": "npm:2024.9.16" "@remixicon/react": "npm:^4.2.0" @@ -3549,7 +3549,7 @@ __metadata: optional: true react-router: optional: true - checksum: 10/c1e802e77d0580b5e04748386bc361779ec250f8093613c72615b53a096928e8021cfdcae2488edeb1eaabfc8114c8f8c9dd73b651ffd248432c91cb6a925e30 + checksum: 10/482ceacb80b281fcb687a700431185dc36615048429ece0391ae0a317cd7aca79544e5970b98dd4da87441178ef187575ffa3df8141745394b412de63f2500c6 languageName: node linkType: hard @@ -13669,7 +13669,7 @@ __metadata: "@ebay/nice-modal-react": "npm:^1.2.13" "@emotion/cache": "npm:^11.13.1" "@faker-js/faker": "npm:^8.4.1" - "@mitodl/course-search-utils": "https://github.com/mitodl/course-search-utils.git#2be637effa7e3bdaf9adcbf98a04724cba2753b1" + "@mitodl/course-search-utils": "npm:3.2.4" "@remixicon/react": "npm:^4.2.0" "@tanstack/react-query": "npm:^4.36.1" "@testing-library/jest-dom": "npm:^6.4.8"