[legacy-framework] (major) Upgrade to React Query v3

[philipj93]

Closes blitz-js/legacy-framework#518

What are the changes and their implications?

This is still a work in progress. Probably some details I'm glossing over, but I wanted to get the ball rolling on this one, hoping someone will have some input for some of the TypeScript errors coming into play. The bulk of changes are happening in @blitzjs/core/src/use-query-hooks.ts.

A couple of the TypeScript errors getting in the way of progressing are mentioned below. Please take a look if you have time! Would love some help on this one. Thanks 😄

Issues:

• Overload function error on useInfiniteQuery (line 155, packages/core/src/use-query-hooks.ts)
• Type conversion error on useReactQueryMutation in useMutation export (line 27, packages/core/src/use-mutation.ts

[flybayer]

yay! If you get blocked on the TS issues, let me know and I can take a look

[philipj93]

yay! If you get blocked on the TS issues, let me know and I can take a look

@flybayer Awesome, I could use help mostly on the mutation hook implementation. The query hooks weren't too bad (just an overload error right now on useInfiniteQuery). Here's what I see for that one:

In packages/core/src/types.ts:

// These were imported with v2
// import {MutateOptions, MutationResult} from "react-query"
// Not sure if this the right type to use with v3
import {UseMutationOptions, UseMutationResult} from "react-query"

Those are used further down in the file:

// Old types with v2
// export declare type MutateFunction<
//   TResult,
//   TError = unknown,
//   TVariables = unknown,
//   TSnapshot = unknown
// > = (
//   variables?: TVariables,
//   config?: MutateOptions<TResult, TError, TVariables, TSnapshot>,
// ) => Promise<TResult>

// export declare type MutationResultPair<TResult, TError, TVariables, TSnapshot> = [
//   MutateFunction<TResult, TError, TVariables, TSnapshot>,
//   MutationResult<TResult, TError>,
// ]

// New types with v3
export declare type MutateFunction<
  TResult,
  TError = unknown,
  TVariables = unknown,
  TSnapshot = unknown
> = (
  variables?: TVariables,
  config?: UseMutationOptions<TResult, TError, TVariables, TSnapshot>,
) => Promise<TResult>

export declare type MutationResultPair<TResult, TError, TVariables, TSnapshot> = [
  MutateFunction<TResult, TError, TVariables, TSnapshot>,
  UseMutationResult<TResult, TError>,
]

That's giving me an error in pacakges/core/src/use-matation.ts that says

Conversion of type 'UseMutationResult<TResult, unknown, TVariables, unknown>' to type 'MutationResultPair<TResult, TError, TVariables, TSnapshot>' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.ts(2352)

Error happens on the return line for the useMutation function:

export function useMutation<TResult, TError = unknown, TVariables = undefined, TSnapshot = unknown>(
  mutationResolver: MutationFunction<TResult, TVariables>,
  config?: MutationOptions<TResult, TError, TVariables, TSnapshot>,
) {
  const enhancedResolverRpcClient = sanitize(mutationResolver)

  return useReactQueryMutation(
    (variables: TVariables) => enhancedResolverRpcClient(variables, {fromQueryHook: true}),
    {
      throwOnError: true,
      ...config,
    } as any,
  ) as MutationResultPair<TResult, TError, TVariables, TSnapshot>
}

Pretty lost though on where to go with this one. Any help would be great 😄

[flybayer]

@tundera I will investigate as soon as I can, but probably not before Monday.

[flybayer]

I got hit with a bunch of urgent client work this week, so haven't been able to write any blitz code this week yet. This is still top priority as soon as I get freed up a bit.

[flybayer]

@tundera I fixed those typescript issues!

Remaining items I'll leave for you:

• Fix internal tests (need to add a query client provider)
• Export useQueryErrorResetBoundary from @blitzjs/core
• Move result of useDefaultQueryConfig to QueryClient.defaultOptions
• Remove useDefaultQueryConfig
• Add the query client providers to BlitzOuterRoot instead of being in user's _app files
• Update new app template and examples for the above the above changes

[philipj93]

@flybayer Awesome, I'm going to try to get those wrapped up this weekend

[philipj93]

@flybayer Hey I had some to time to work on these issues this weekend and here's what I was able to get done:

• Fix internal tests (need to add a query client provider)
• Export useQueryErrorResetBoundary from @blitzjs/core
• Move result of useDefaultQueryConfig to QueryClient.defaultOptions
• Remove useDefaultQueryConfig
• Add the query client providers to BlitzOuterRoot instead of being in user's _app files
• Update new app template and examples for the above the above changes

Still having trouble on some tests in the core package. Not sure if that's because the query client provider isn't set up correctly in the test utils file, but that's my hunch. The tests that seem to fail are in the use-query.test.tsx file and are related to suspense. Take a look when you get a chance and hopefully you'll notice if there are any issues that stand out. Thanks!

[flybayer]

@tundera got those issues fixed!

Now last code issue is jest tests inside user's apps. Looks like we should export a <BlitzProvider> component that people can add to their test-utils. (we can use this later to also include a mock router provider)

And then the last thing is making a PR to the docs repo to update all our docs :)

[flybayer]

Awesome, looking good! Thanks for sticking with me on this!

[flybayer]

And looks like we have some failing tests

[flybayer]

Ok yeah let's keep usePaginatedQuery. Sorry, I probably should have specified that but you kept it initially. And I didn't think about you removing it later.

It provides a better DX, plus it's less breaking changes for everyone.

[philipj93]

@flybayer Cool, I added it back in . Getting a failing windows test in CI but having a hard time understanding what's causing it

[flybayer]

Awesome, thank you so much!!!

[blitzjs-bot]

Added @tundera contributions for test