diff --git a/jest.config.ts b/jest.config.ts index 41f2f5e..9cfc46c 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -27,4 +27,4 @@ const config: Config.InitialOptions = { ] } -export default config \ No newline at end of file +export default config diff --git a/package.json b/package.json index 5e4626f..ba25c8f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@mitodl/course-search-utils", - "version": "2.0.4", + "version": "2.1.0", "description": "JS utils for interacting with MIT Open Course search", "main": "dist/index.js", "files": [ @@ -41,6 +41,7 @@ "devDependencies": { "@swc/core": "^1.3.0", "@swc/jest": "^0.2.22", + "@testing-library/react-hooks": "^8.0.1", "@types/enzyme": "^3.10.7", "@types/jest": "^29.0.1", "@types/lodash": "^4.14.162", diff --git a/src/hooks.ts b/src/hooks.ts index a109d7d..f8c5034 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -1,14 +1,17 @@ -import { useEffect, useRef } from "react" +import { useEffect, useState } from "react" -export function useDidMountEffect(fn: () => void, deps: any[]): void { - const renderedOnce = useRef(false) +/** + * Like `useEffect`, but only runs after component has rendered at least once. + */ +export function useEffectAfterMount(fn: () => void, deps: any[]): void { + const [hasRendered, setHasRendered] = useState(false) useEffect(() => { - if (renderedOnce.current) { + if (hasRendered) { fn() } else { - renderedOnce.current = true + setHasRendered(true) } // eslint-disable-next-line react-hooks/exhaustive-deps - }, deps) + }, [hasRendered, ...deps]) } diff --git a/src/index.test.tsx b/src/index.test.tsx index 549a035..1977a15 100644 --- a/src/index.test.tsx +++ b/src/index.test.tsx @@ -1,7 +1,12 @@ import * as React from "react" import { mount } from "enzyme" import { act } from "react-dom/test-utils" -import { MemoryHistoryOptions, createMemoryHistory } from "history" +import { renderHook } from "@testing-library/react-hooks/dom" +import { + MemoryHistoryOptions, + createMemoryHistory, + InitialEntry +} from "history" import { LearningResourceType, @@ -9,7 +14,7 @@ import { LR_TYPE_ALL } from "./constants" -import { useCourseSearch } from "./index" +import { useCourseSearch, useSearchInputs, useSyncUrlAndSearch } from "./index" import { facetMap, wait } from "./test_util" import { serializeSort, serializeSearchParams } from "./url_utils" @@ -309,14 +314,18 @@ describe("useCourseSearch", () => { const { wrapper, runSearch } = render() act(() => { // @ts-expect-error - wrapper.find(".toggleFacet").prop("onClick")("topic", "mathematics", true) + wrapper.find(".toggleFacet").prop("onClick")( + "topics", + "mathematics", + true + ) }) checkSearchCall(runSearch, [ "", { ...INITIAL_FACET_STATE, - type: LR_TYPE_ALL, - topic: ["mathematics"] + type: LR_TYPE_ALL, + topics: ["mathematics"] }, 0, null, @@ -329,7 +338,7 @@ describe("useCourseSearch", () => { act(() => { // @ts-expect-error wrapper.find(".toggleFacets").prop("onClick")([ - ["topic", "mathematics", true], + ["topics", "mathematics", true], ["type", LearningResourceType.Course, false], ["type", LearningResourceType.ResourceFile, true] ]) @@ -338,8 +347,8 @@ describe("useCourseSearch", () => { "", { ...INITIAL_FACET_STATE, - type: [LearningResourceType.ResourceFile], - topic: ["mathematics"] + type: [LearningResourceType.ResourceFile], + topics: ["mathematics"] }, 0, null, @@ -519,3 +528,391 @@ describe("useCourseSearch", () => { ]) }) }) + +describe("useSearchInputs", () => { + it.each([ + { + descrip: "initial: empty", + initial: null, + expected: { + text: "", + activeFacets: INITIAL_FACET_STATE, + sort: null, + ui: null + } + }, + { + descrip: "initial: text, facets, ui", + initial: { + text: "cat", + activeFacets: { topics: ["math", "bio"] }, + ui: "list" + }, + expected: { + text: "cat", + activeFacets: { ...INITIAL_FACET_STATE, topics: ["math", "bio"] }, + ui: "list", + sort: null + } + }, + { + descrip: "initial: sort", + initial: { sort: { field: "coursenum", option: "asc" } }, + expected: { + text: "", + activeFacets: INITIAL_FACET_STATE, + sort: { field: "coursenum", option: "asc" }, + ui: null + } + } + ])( + "Reads initial searchParams and text from history ($desc)", + ({ initial, expected }) => { + const initialEntries = initial ? + [`?${serializeSearchParams(initial)}`] : + undefined + const history = createMemoryHistory({ initialEntries }) + const { result } = renderHook(() => useSearchInputs(history)) + expect(result.current.searchParams).toEqual(expected) + expect(result.current.text).toEqual(expected.text) + + /** + * We want to ensure that the initial state is set from the URL, as + * opposed to the initial state being empty and updated via a useEffect + * hook. The latter approach could lead to extra runSearch calls in + * useCourseSearch, or extra history entries with useSyncUrlAndSearch + */ + expect(result.all.length).toBe(1) + } + ) + + test("setSearchParams updates searchParams when given an object", () => { + const history = createMemoryHistory() + const { result } = renderHook(() => useSearchInputs(history)) + act(() => { + result.current.setSearchParams({ + text: "cat", + activeFacets: { topics: ["math", "bio"] }, + ui: null, + sort: null + }) + }) + expect(result.current.searchParams).toEqual({ + text: "cat", + activeFacets: { topics: ["math", "bio"] }, + sort: null, + ui: null + }) + }) + + test("setSearchParams updates searchParams when given an callback", () => { + const history = createMemoryHistory() + const { result } = renderHook(() => useSearchInputs(history)) + act(() => { + result.current.setSearchParams(current => ({ ...current, text: "cat" })) + }) + expect(result.current.searchParams).toEqual({ + text: "cat", + activeFacets: INITIAL_FACET_STATE, + sort: null, + ui: null + }) + }) + + test("setText updates text but NOT searchParams", () => { + const history = createMemoryHistory() + const { result } = renderHook(() => useSearchInputs(history)) + expect(result.current.text).toEqual("") + const initialSearchParams = result.current.searchParams + act(() => { + result.current.setText("cat") + }) + expect(result.current.searchParams).toEqual(initialSearchParams) + expect(result.current.text).toEqual("cat") + }) + + test("clearAllFilters clears text and searchParams", () => { + const initialEntries = [ + `?${serializeSearchParams({ + text: "cat", + activeFacets: { topics: ["math", "bio"] }, + ui: "list", + sort: { field: "coursenum", option: "asc" } + })}` + ] + const history = createMemoryHistory({ initialEntries }) + const { result } = renderHook(() => useSearchInputs(history)) + const initialSearchParams = result.current.searchParams + const initialText = result.current.text + + act(() => result.current.clearAllFilters()) + // data changed + expect(result.current.searchParams).not.toEqual(initialSearchParams) + expect(result.current.text).not.toEqual(initialText) + + // reset as expected + expect(result.current.searchParams).toEqual({ + text: "", + activeFacets: INITIAL_FACET_STATE, + sort: null, + ui: null + }) + expect(result.current.text).toEqual("") + }) + + test("toggleFacet adds/removes a facet", () => { + const initialEntries = [ + `?${serializeSearchParams({ + text: "cat", + activeFacets: { topics: ["math", "bio"] } + })}` + ] + const history = createMemoryHistory({ initialEntries }) + const { result } = renderHook(() => useSearchInputs(history)) + act(() => { + result.current.toggleFacet("topics", "math", false) + }) + expect(result.current.searchParams.activeFacets.topics).toEqual(["bio"]) + act(() => { + result.current.toggleFacet("topics", "math", true) + }) + expect(result.current.searchParams.activeFacets.topics).toEqual([ + "bio", + "math" + ]) + }) + + test("toggleFacets adds/removes multiple facets", () => { + const initialEntries = [ + `?${serializeSearchParams({ + text: "cat", + activeFacets: { + topics: ["math", "bio"], + level: ["beginner"], + department_name: ["physics", "biology"] + } + })}` + ] + const history = createMemoryHistory({ initialEntries }) + const { result } = renderHook(() => useSearchInputs(history)) + act(() => { + result.current.toggleFacets([ + ["topics", "chem", true], + ["level", "beginner", false], + ["department_name", "chemistry", true] + ]) + }) + expect(result.current.searchParams.activeFacets.topics).toEqual([ + "math", + "bio", + "chem" + ]) + expect(result.current.searchParams.activeFacets.level).toEqual([]) + expect(result.current.searchParams.activeFacets.department_name).toEqual([ + "physics", + "biology", + "chemistry" + ]) + }) + + test("onUpdateFacet adds/removes a facet", () => { + const initialEntries = [ + `?${serializeSearchParams({ + text: "cat", + activeFacets: { topics: ["math", "bio"] } + })}` + ] + const history = createMemoryHistory({ initialEntries }) + const { result } = renderHook(() => useSearchInputs(history)) + act(() => { + result.current.onUpdateFacet({ + target: { name: "topics", value: "math", checked: false } + }) + }) + expect(result.current.searchParams.activeFacets.topics).toEqual(["bio"]) + act(() => { + result.current.onUpdateFacet({ + target: { name: "topics", value: "math", checked: true } + }) + }) + expect(result.current.searchParams.activeFacets.topics).toEqual([ + "bio", + "math" + ]) + }) + + test("toggleFacet sets texts -> searchParams.text", () => { + const history = createMemoryHistory() + const { result } = renderHook(() => useSearchInputs(history)) + + act(() => result.current.setText("algebra")) + + expect(result.current.searchParams.text).toEqual("") + act(() => { + result.current.toggleFacet("topics", "math", true) + }) + expect(result.current.searchParams.text).toEqual("algebra") + }) + + test("toggleFacets sets texts -> searchParams.text", () => { + const history = createMemoryHistory() + const { result } = renderHook(() => useSearchInputs(history)) + + act(() => result.current.setText("algebra")) + + expect(result.current.searchParams.text).toEqual("") + act(() => { + result.current.toggleFacets([["topics", "math", true]]) + }) + expect(result.current.searchParams.text).toEqual("algebra") + }) + + test("onUpdateFacet sets texts -> searchParams.text", () => { + const history = createMemoryHistory() + const { result } = renderHook(() => useSearchInputs(history)) + + act(() => result.current.setText("algebra")) + + expect(result.current.searchParams.text).toEqual("") + act(() => { + result.current.onUpdateFacet({ + target: { name: "topics", value: "math", checked: true } + }) + }) + expect(result.current.searchParams.text).toEqual("algebra") + }) + + test("onUpdateText updates texts but not searchParams.text; submitText finalizes text", () => { + const history = createMemoryHistory() + const { result } = renderHook(() => useSearchInputs(history)) + + act(() => result.current.setText("algebra")) + expect(result.current.text).toBe("algebra") + expect(result.current.searchParams.text).toBe("") + + act(() => result.current.submitText()) + expect(result.current.searchParams.text).toBe("algebra") + }) + + test("clearText clears text and searchParams.text", () => { + const history = createMemoryHistory({ + initialEntries: [`?${serializeSearchParams({ text: "algebra" })}`] + }) + const { result } = renderHook(() => useSearchInputs(history)) + + expect(result.current.text).toBe("algebra") + expect(result.current.searchParams.text).toBe("algebra") + + act(() => result.current.clearText()) + expect(result.current.text).toBe("") + expect(result.current.searchParams.text).toBe("") + }) + + test("updateSort updates sort and sets text -> searchParams.text", () => { + const history = createMemoryHistory() + const { result } = renderHook(() => useSearchInputs(history)) + + act(() => result.current.setText("algebra")) + + expect(result.current.searchParams.sort).toBe(null) + expect(result.current.searchParams.text).toBe("") + + act(() => { + result.current.updateSort({ target: { value: "-title" } }) + }) + expect(result.current.searchParams.sort).toEqual({ + field: "title", + option: "desc" + }) + expect(result.current.searchParams.text).toBe("algebra") + }) + + test("updateUI updates ui and sets text -> searchParams.text", () => { + const history = createMemoryHistory() + const { result } = renderHook(() => useSearchInputs(history)) + + act(() => result.current.setText("algebra")) + + expect(result.current.searchParams.sort).toBe(null) + expect(result.current.searchParams.text).toBe("") + + act(() => { + result.current.updateUI("list") + }) + expect(result.current.searchParams.ui).toBe("list") + expect(result.current.searchParams.text).toBe("algebra") + }) +}) + +describe("useSyncUrlAndSearch", () => { + const setupHook = (initialEntries?: InitialEntry[]) => { + const history = createMemoryHistory({ initialEntries }) + const all = renderHook(() => { + const search = useSearchInputs(history) + useSyncUrlAndSearch(history, search) + return search + }) + return [all, history] as const + } + + it("syncs searchParams to url", async () => { + const [{ result }, history] = setupHook(["/search"]) + + expect(history.index).toBe(0) + act(() => { + result.current.setText("algebra") + result.current.toggleFacet("topics", "math", true) + result.current.submitText() + }) + + act(() => { + result.current.setText("linear algebra") + }) + + expect(result.current.searchParams.text).toBe("algebra") + expect(result.current.text).toBe("linear algebra") // not submitted yet + + expect(history.location).toEqual( + expect.objectContaining({ + pathname: "/search", + search: "?q=algebra&t=math" + }) + ) + expect(history.index).toBe(1) + }) + + it("syncs empty search parameters to URL correctly", () => { + const [{ result }, history] = setupHook(["/search?q=algebra"]) + act(() => { + result.current.clearText() + }) + expect(history.location).toEqual( + expect.objectContaining({ + pathname: "/search", + search: "" + }) + ) + }) + + it("syncs url to searchParams", async () => { + const [{ result }, history] = setupHook([ + `?${serializeSearchParams({ + text: "algebra", + activeFacets: { topics: ["math"] } + })}` + ]) + + act(() => { + result.current.clearAllFilters() + }) + + expect(result.current.searchParams.text).toBe("") + expect(history.location.search).toBe("") + + act(() => history.go(-1)) + + expect(history.location.search).toBe("?q=algebra&t=math") + expect(result.current.searchParams.text).toBe("algebra") + expect(result.current.searchParams.activeFacets.topics).toEqual(["math"]) + }) +}) diff --git a/src/index.ts b/src/index.ts index 105ef8c..3e8842e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,7 +3,8 @@ import React, { useCallback, useEffect, MouseEvent, - ChangeEvent + useMemo, + useRef } from "react" import { unionWith, eqBy, prop, clone } from "ramda" import _ from "lodash" @@ -20,9 +21,10 @@ import { deserializeSearchParams, deserializeSort, serializeSearchParams, - SortParam + SortParam, + SearchParams } from "./url_utils" -import { useDidMountEffect } from "./hooks" +import { useEffectAfterMount } from "./hooks" export * from "./constants" @@ -70,72 +72,13 @@ const history4or5Listen = ( }) } -interface PreventableEvent { - preventDefault?: () => void - type?: string -} -interface CourseSearchResult { - facetOptions: (group: string) => Aggregation | null - clearAllFilters: () => void - toggleFacet: (name: string, value: string, isEnbaled: boolean) => void - toggleFacets: (facets: [string, string, boolean][]) => void - onUpdateFacets: React.ChangeEventHandler - updateText: React.ChangeEventHandler - clearText: React.MouseEventHandler - updateSort: React.ChangeEventHandler - acceptSuggestion: (suggestion: string) => void - loadMore: () => void - incremental: boolean - text: string - sort: SortParam | null - activeFacets: Facets - /** - * Callback that handles search submission. Pass this to your search input - * submission event target, e.g., `
` or - * `