fix history dep #59
fix history dep #59
Conversation
ChristopherChudzicki
force-pushed
the
cc/fix-history-dep
branch
from
a483652
to
083bccb
Compare
| @@ -8,7 +8,7 @@ import React, { | |||
| } from "react" | |||
| import { unionWith, eqBy, prop, clone } from "ramda" | |||
| import _ from "lodash" | |||
| import { History as HHistory } from "history" | |||
| import type { History as HHistory } from "history" | |||
There was a problem hiding this comment.
import type statements are removed when Typescript is compiled to Javascript.
A consuming typescript project that uses history v5 will use types directly from history, which provides its own types starting in v5.
A consuming typescript project that uses history v4 will use types from @types/history.
A consuming javascript project will not use types.
| @@ -431,11 +431,10 @@ export const useCourseSearch = ( | |||
| updateUI | |||
| } = seachUI | |||
| const { activeFacets, sort, ui } = searchParams | |||
| const activeFacetsAndSort = useMemo(() => ({ activeFacets, sort, ui }), [ | |||
There was a problem hiding this comment.
Changed by prettier v1 -> v2 upgrade.
| "@types/history": "^4.9", | ||
| "history": "^4.9 || ^5.0.0", |
There was a problem hiding this comment.
The only way that course-search-utils uses history is as a type annotation for the useCourseSearch export, which has a history object as one of its argument. This library does use history directly.
history should be a peer dependency because we want to be sure that course-search-utils and the parent app are using the same version of history. Additionally, it's an optional peer dependency because: if you're not using the useCourseSearch export, you don't need history in your app.
@types/history is optional for the same reason. Additionally, you don't need @types/history if you're not using typescript, or if you're using history v5, which includes its own types.
|
Looks like this breaks infinite search for me, specifically when you use the back button to go back to a search that has text. If you
the page gets stuck in the loading state and a new search request is never made |
|
This issue @abeglova mentioned was fixed by a As an aside: This should be very safe. There are no non-linting changes to the package code. |
Relevant Tickets
#58, mitodl/open-discussions#4115
Description (What does it do?)
This PR:
historyfrom a dependency to a peer dependency.prettier, since we needed toimport type { ... } from "history", andimport typeis not supported by Prettier v1See code comments.
How should this be tested?
In Open discussions:
git checkout cc/update-course-search-utils docker compose run --rm watch yarn install # or docker-compose upCheck that search still works.