EVG-16587 Bump Apollo and graphql deps #1365
Conversation
| @@ -189,7 +189,6 @@ | |||
| "eslint-plugin-storybook": "0.5.6", | |||
There was a problem hiding this comment.
(placement of comment is random)
Could we possibly remove @types/graphql from devDependencies?
| const [ | ||
| getVersion, |
There was a problem hiding this comment.
Moved this above its usage on line 73 due to it becoming undefined. This was likely a result of one of the various useLazyQuery refactors that happened between our updates.
| ] = useLazyQuery<VersionQuery, VersionQueryVariables>(GET_VERSION, { | ||
| variables: { id }, | ||
| pollInterval, | ||
| fetchPolicy: "cache-and-network", |
There was a problem hiding this comment.
Removed unnecessary fetchNextPolicy thats behavior was updated in apollographql/apollo-client#8465
| @@ -310,7 +310,9 @@ const attachProjectToRepoMock = { | |||
| }, | |||
| result: { | |||
| data: { | |||
| id: "evergreen", | |||
There was a problem hiding this comment.
These mocks were invalid and were fixed.
| fetchPolicy: "network-only", | ||
| nextFetchPolicy: "cache-and-network", | ||
| skip: !hasQueryVariables, | ||
| fetchPolicy: "cache-and-network", |
There was a problem hiding this comment.
nextFetchPolicy had inconsistent behavior which was fixed in a subsequent update. nextFetchPolicy would revert to the original fetchPolicy after it is called. I updated the fetchPolicy as well as updated the loading state of the table. Imo it works better than before. and is more in line with the loading behavior on the LG table
src/pages/version/Tasks.tsx
Outdated
| <PatchTasksTable | ||
| sorts={sorts} | ||
| tasks={tasks} | ||
| loading={tasks.length === 0} |
There was a problem hiding this comment.
I think this offers the best compromise between loading behavior and user experience.
If we have no data about the tasks we show the loading page but if we had previously visited a page we query in the background while showing the user somewhat stale data. This makes pagination seemless.
|
LGTM |
src/pages/Version.tsx
Outdated
| ); | ||
| setIsLoadingData(false); | ||
| }, | ||
| }); | ||
| usePolling(startPolling, stopPolling, refetch, false); |
There was a problem hiding this comment.
Can you move usePolling up as well? The hook is always called right after query definitions, and it would be good to be consistent
src/pages/Version.tsx
Outdated
| console.log("Calling getVersion"); | ||
| console.log({ getVersion }); |
src/pages/version/Tasks.tsx
Outdated
| onError: (err) => { | ||
| dispatchToast.error(`Error fetching patch tasks ${err}`); | ||
| }, | ||
| }); | ||
| usePolling(startPolling, stopPolling, refetch); | ||
| const { patchTasks } = data || {}; | ||
| const { tasks } = patchTasks || {}; | ||
|
|
||
| const { tasks } = patchTasks || { tasks: [] }; |
There was a problem hiding this comment.
alternatively const { tasks =[] } = patchTasks || {}, which would be parallel to what we have in TaskDuration.tsx
| } | ||
|
|
||
| export const PatchTasksTable: React.VFC<Props> = ({ tasks, sorts }) => { | ||
| export const PatchTasksTable: React.VFC<Props> = ({ | ||
| tasks = [], |
| @@ -28,7 +27,7 @@ export const Tasks: React.VFC<Props> = ({ taskCount }) => { | |||
| const updateQueryParams = useUpdateURLQueryParams(); | |||
|
|
|||
| const queryVariables = useQueryVariables(search, id); | |||
| const noQueryVariables = !Object.keys(parseQueryString(search)).length; | |||
| const hasQueryVariables = Object.keys(parseQueryString(search)).length > 0; | |||
There was a problem hiding this comment.
might be worth also changing the one in TaskDuration.tsx to look like this
| <PatchTasksTable | ||
| sorts={sorts} | ||
| tasks={tasks} | ||
| loading={tasks.length === 0 && loading} |
EVG-16587
Description
Bumps Apollo/client and graphql to latest versions.
Deletes unused graphql-tag dep