add useSearchQueryParams and useInfiniteSearch

[ChristopherChudzicki]

What are the relevant tickets?

#65

Description (What does it do?)

The goal of this PR is to make this library easier for consuming apps to use and simplify the implementation of some of the hooks. I add two hooks:

1. useSearchQueryParams: derive search API parameters from the URLSearchParams and return functions to help update URLSearchParams
2. useInfiniteSearch: make paginated search API calls use search params returned by previous hook.

How can this be tested?

To see the hooks in use (outside of tests), I made a branch in MIT Open using the hooks in this PR https://github.com/mitodl/mit-open/tree/cc/search-page-experiment. (It doesn't directly use this branch, it uses this branch cherry-picked to #69).

In MIT Open repo:

1. git fetch, git checkout cc/search-page-experiment.
2. View SearchPage.tsx code
3. View http://localhost:8063/search/ and try out the page: filtering facets, scrolling, etc.
   • facet state should update
   • URL should update
   • search results should be accurate
   • inspect API calls in network tab

trimmed_a_Bit.mov

[ChristopherChudzicki]

"Infinite" here means "infinite scroll"; no association with "infinite corridor"

Not clear to me that this hook would actually be used in MIT Open since the current designs have paginated UI instead of infinite scroll.

[ChristopherChudzicki]

Very open to a better name that useSearchQueryParams.

• useSearchParams is the name of react-router's hook for accesing the URLSearchParams object
• in general, there's some naming difficulty because search API vs query params (aka search params) in URL

[jonkafton]

The naming from the html standard is search from Window.location and URLSearchParams. If we don't want to reuse useSearchParams and alias on import, I'd go useCourseSearchParams.

Perhaps it should return an instance of URLSearchParams if we want to align them.

[ChristopherChudzicki]

Right, I felt the trouble with "search" is that it's too overloaded with the API being search related.

Perhaps it should return an instance of URLSearchParams if we want to align them.

I see this hook as primarily making the interface of URLSearchParams friendlier: Returning validated params as an object (with their full names rather than aliases, i.e., "department" not "d") and settings that help manipulate URLSearchParams.

I'd go useCourseSearchParams
MIT Open has two search endpoints:

• https://mitopen-rc.odl.mit.edu/api/v1/learning_resources_search/ (Courses, podcasts, programs, ...)
• https://mitopen-rc.odl.mit.edu/api/v1/content_file_search/ (HTML docs, pdfs, etc; usually materials within a course)

and the hook helps with both (which is controlled via "endpoint").

Having one hook do both might make things like https://ocw.mit.edu/search easier (note the "Courses" and "resources" tabs. The lingo is a bit confusing. In MIT Open's lingo, those tabs would be "Resources" and "Content Files").

Having one hook do both also makes the typescript a lot looser. I could separate them.

[jonkafton]

I do see the naming dilemma. The "course" in useCourseSearchParams aligns with the package name - perhaps it needs a more generic name.

[jonkafton]

The mit-open cc/search-page-experiment branch is pointing to #a335d56 on cc/combined - should this be the head branch for this PR? I needed the files from https://github.com/mitodl/course-search-utils/tree/cc/combined/src/facet_display.

[jonkafton]

Not related to this PR, but let's swap out ramda and lodash.

[jonkafton]

I might be missing the purpose, but can we abstract further to only import useInfiniteSearch into our component and pass it params from react-routers's useParams()? useInfiniteSearch then defaults to using useSearchQueryParams and returns setFacetActive, clearFacets, etc.

Are there likely to be cases where we don't want location.search to update - we could pass setSearchParams optionally, or just a boolean to instruct it to update the search URL or not. The consuming component can listen to URL changes only if it needs them.

[jonkafton]

Can we reuse Facets from facet_display/types.ts aside from the booleans (though these are currently missing the generated API enums)?

[ChristopherChudzicki]

Not related to this PR, but let's swap out ramda and lodash.

Very much agree. #48

[ChristopherChudzicki]

Are there likely to be cases where we don't want location.search to update - we could pass setSearchParams optionally, or just a boolean to instruct it to update the search URL or not. The consuming component can listen to URL changes only if it needs them

I think if you don't want location.search to update when facets or search text change, you also shouldn't use it as the source of truth for search parameters. In that case, do what I did in the tests:

const [searchParams, setSearchParams] = useState(new URLSearchParams())
const { params } = useSearchQueryParams({  searchParams, setSearchParams })

Then the state is all internal to react and not synced with URL.

I might be missing the purpose, but can we abstract further to only import useInfiniteSearch into our component and pass it params from react-routers's useParams()? useInfiniteSearch then defaults to using useSearchQueryParams and returns setFacetActive, clearFacets, etc.

We could do this. I think it would be best done as a separate hook, for two reasons.

First, a use-case that we have already is adding a few extra facets that do NOT come from the URL. For example, https://ocw.mit.edu/search silently sets platform=ocw on all search requests. With separate hooks, that can be done via

const [searchParams, setSearchParams] = useState(new URLSearchParams())
const { params: partialParams } = useSearchQueryParams({  searchParams, setSearchParams })
const params = useMemo(() => mergeParams(partialParams, SOME_CONSTANT_PARAMS), [partialParams])
const { pages } = useInfiniteSearch({ params, ... })

Of course, we could have a paramsOverrides or something option. But using separate hooks leaves that flexibility and keeps the hook interfaces simple.

Second, we might not want infinite scroll. Maybe we want pagination.

[jonkafton]

It works very nicely, looks clean and is well tested. The approach seems sensible - no changes from me.

A request though to write up usage notes in the readme with some of the thinking discussed above.