Repository: tannerlinsley/react-query (ts)

Issue:
memoized selectors and errors
**Describe the bug**

If a memoized selector throws an error, the selector function is not re-executed again. Instead, the query goes to `isSuccess` state with  `undefined` data and `error: null`

select error handling was introduced here: https://github.com/tannerlinsley/react-query/pull/1760
selector memoization was introduced here: https://github.com/tannerlinsley/react-query/pull/1751

**To Reproduce**

see: https://codesandbox.io/s/peaceful-kare-8qs3b?file=/src/index.js

if you inline the selector, it works as expected.

**Expected behavior**
The query should always go to `isError` state 

/cc @boschni 



Pull request:
fix(queryObserver): re-run selector if it throws error
fixes #1855 

This ensures that the selector function is re-executed, and the catch block is reached and error is handled, leading to the correct isError state.

Patch:
diff --git a/README.md b/README.md
index 1ff558d1d..1eae451f6 100644
--- a/README.md
+++ b/README.md
@@ -81,4 +81,4 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
 
 This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome!
 
-<!-- Force 2 -->
+<!-- Force 3 -->
diff --git a/docs/src/pages/guides/initial-query-data.md b/docs/src/pages/guides/initial-query-data.md
index c15f024b6..8466322f0 100644
--- a/docs/src/pages/guides/initial-query-data.md
+++ b/docs/src/pages/guides/initial-query-data.md
@@ -99,7 +99,7 @@ function Todo({ todoId }) {
 
 ### Initial Data from the cache with `initialDataUpdatedAt`
 
-Getting initial data from the cache means the source query you're using to look up the initial data from is likely old, but `initialData` Instead of using an artificial `staleTime` to keep your query from refetching immediately, it's suggested that you pass the source query's `dataUpdatedAt` to `initialDataUpdatedAt`. This provides the query instance with all the information it needs to determine if and when the query needs to be refetched, regardless of initial data being provided.
+Getting initial data from the cache means the source query you're using to look up the initial data from is likely old, but `initialData`. Instead of using an artificial `staleTime` to keep your query from refetching immediately, it's suggested that you pass the source query's `dataUpdatedAt` to `initialDataUpdatedAt`. This provides the query instance with all the information it needs to determine if and when the query needs to be refetched, regardless of initial data being provided.
 
 ```js
 function Todo({ todoId }) {
diff --git a/docs/src/pages/guides/mutations.md b/docs/src/pages/guides/mutations.md
index 769e7f806..498c6f690 100644
--- a/docs/src/pages/guides/mutations.md
+++ b/docs/src/pages/guides/mutations.md
@@ -149,7 +149,7 @@ useMutation(addTodo, {
 })
 ```
 
-You might find that you want to **trigger additional callbacks** then the ones defined on `useMutation` when calling `mutate`. This can be used to trigger component specific side effects. To do that, you can provide any of the same callback options to the `mutate` function after your mutation variable. Supported overrides include: `onSuccess`, `onError` and `onSettled`.
+You might find that you want to **trigger additional callbacks** than the ones defined on `useMutation` when calling `mutate`. This can be used to trigger component-specific side effects. To do that, you can provide any of the same callback options to the `mutate` function after your mutation variable. Supported overrides include: `onSuccess`, `onError` and `onSettled`.
 
 ```js
 useMutation(addTodo, {
diff --git a/docs/src/pages/plugins/createLocalStoragePersistor.md b/docs/src/pages/plugins/createLocalStoragePersistor.md
index 786dfe8b7..d0005e5b0 100644
--- a/docs/src/pages/plugins/createLocalStoragePersistor.md
+++ b/docs/src/pages/plugins/createLocalStoragePersistor.md
@@ -30,7 +30,7 @@ const queryClient = new QueryClient({
 const localStoragePersistor = createLocalStoragePersistor()
 
 persistQueryClient({
-  queryClient
+  queryClient,
   persistor: localStoragePersistor,
 })
 ```
diff --git a/docs/src/pages/reference/QueryClientProvider.md b/docs/src/pages/reference/QueryClientProvider.md
index f956dccf6..e31338de6 100644
--- a/docs/src/pages/reference/QueryClientProvider.md
+++ b/docs/src/pages/reference/QueryClientProvider.md
@@ -14,3 +14,11 @@ function App() {
   return <QueryClientProvider client={queryClient}>...</QueryClientProvider>
 }
 ```
+**Options**
+
+- `client: QueryClient`
+  - **Required**
+  - the QueryClient instance to provide
+- `contextSharing: boolean`
+  - defaults to `false`
+  - Set this to `true` to enable context sharing, which will share the first and at least one instance of the context across the window  to ensure that if React Query is used across  different bundles or microfrontends they will  all use the same **instance** of context, regardless of module scoping.
diff --git a/docs/src/pages/reference/useQuery.md b/docs/src/pages/reference/useQuery.md
index f0fe43d06..90d1659cc 100644
--- a/docs/src/pages/reference/useQuery.md
+++ b/docs/src/pages/reference/useQuery.md
@@ -11,6 +11,7 @@ const {
   errorUpdatedAt,
   failureCount,
   isError,
+  isFetched,
   isFetchedAfterMount,
   isFetching,
   isIdle,
@@ -201,6 +202,8 @@ const result = useQuery({
   - Will be `true` if the data shown is the placeholder data.
 - `isPreviousData: boolean`
   - Will be `true` when `keepPreviousData` is set and data from the previous query is returned.
+- `isFetched: boolean`
+  - Will be `true` if the query has been fetched.
 - `isFetchedAfterMount: boolean`
   - Will be `true` if the query has been fetched after the component mounted.
   - This property can be used to not show any previously cached data.
diff --git a/examples/nextjs/components/PostList/index.js b/examples/nextjs/components/PostList/index.js
index f8b5fcaa8..7c74cdf4f 100644
--- a/examples/nextjs/components/PostList/index.js
+++ b/examples/nextjs/components/PostList/index.js
@@ -1,9 +1,9 @@
 import React, { useState } from 'react'
 import { usePosts } from '../../hooks/usePosts'
 
-export const PostList = ({ isClient }) => {
+export const PostList = () => {
   const [postCount, setPostCount] = useState(10)
-  const { data, isLoading, isFetching } = usePosts(postCount, isClient)
+  const { data, isLoading, isFetching } = usePosts(postCount)
 
   if (isLoading) return <div>Loading</div>
 
diff --git a/src/core/queryClient.ts b/src/core/queryClient.ts
index a7230fbfb..cc7c5427b 100644
--- a/src/core/queryClient.ts
+++ b/src/core/queryClient.ts
@@ -207,7 +207,7 @@ export class QueryClient {
     const refetchFilters: QueryFilters = {
       ...filters,
       active: filters.refetchActive ?? true,
-      inactive: filters.refetchInactive ?? false,
+      inactive: filters.refetchInactive,
     }
 
     return notifyManager.batch(() => {
diff --git a/src/core/queryObserver.ts b/src/core/queryObserver.ts
index f88b4e791..061091ab8 100644
--- a/src/core/queryObserver.ts
+++ b/src/core/queryObserver.ts
@@ -57,6 +57,7 @@ export class QueryObserver<
     TQueryData
   >
   private previousQueryResult?: QueryObserverResult<TData, TError>
+  private previousSelectError: Error | null
   private staleTimeoutId?: number
   private refetchIntervalId?: number
   private trackedProps!: Array<keyof QueryObserverResult>
@@ -70,6 +71,7 @@ export class QueryObserver<
     this.client = client
     this.options = options
     this.trackedProps = []
+    this.previousSelectError = null
     this.bindMethods()
     this.setOptions(options)
   }
@@ -414,7 +416,8 @@ export class QueryObserver<
       if (
         prevResult &&
         state.data === prevResultState?.data &&
-        options.select === prevResultOptions?.select
+        options.select === prevResultOptions?.select &&
+        !this.previousSelectError
       ) {
         data = prevResult.data
       } else {
@@ -423,9 +426,11 @@ export class QueryObserver<
           if (options.structuralSharing !== false) {
             data = replaceEqualDeep(prevResult?.data, data)
           }
+          this.previousSelectError = null
         } catch (selectError) {
           getLogger().error(selectError)
           error = selectError
+          this.previousSelectError = selectError
           errorUpdatedAt = Date.now()
           status = 'error'
         }
diff --git a/src/core/types.ts b/src/core/types.ts
index f3c6eeee9..0545b2976 100644
--- a/src/core/types.ts
+++ b/src/core/types.ts
@@ -3,7 +3,7 @@ import type { QueryBehavior } from './query'
 import type { RetryValue, RetryDelayValue } from './retryer'
 import type { QueryFilters } from './utils'
 
-export type QueryKey = string | unknown[]
+export type QueryKey = string | readonly unknown[]
 
 export type QueryFunction<T = unknown> = (
   context: QueryFunctionContext<any>
@@ -448,7 +448,7 @@ export type InfiniteQueryObserverResult<TData = unknown, TError = unknown> =
   | InfiniteQueryObserverRefetchErrorResult<TData, TError>
   | InfiniteQueryObserverSuccessResult<TData, TError>
 
-export type MutationKey = string | unknown[]
+export type MutationKey = string | readonly unknown[]
 
 export type MutationStatus = 'idle' | 'loading' | 'success' | 'error'
 
@@ -463,7 +463,7 @@ export interface MutationOptions<
   TContext = unknown
 > {
   mutationFn?: MutationFunction<TData, TVariables>
-  mutationKey?: string | unknown[]
+  mutationKey?: MutationKey
   variables?: TVariables
   onMutate?: (variables: TVariables) => Promise<TContext> | TContext
   onSuccess?: (
diff --git a/src/core/utils.ts b/src/core/utils.ts
index 7b7870d07..49cb0b7d6 100644
--- a/src/core/utils.ts
+++ b/src/core/utils.ts
@@ -223,8 +223,8 @@ export function stableValueHash(value: any): string {
  * Checks if key `b` partially matches with key `a`.
  */
 export function partialMatchKey(
-  a: string | unknown[],
-  b: string | unknown[]
+  a: QueryKey,
+  b: QueryKey
 ): boolean {
   return partialDeepEqual(ensureArray(a), ensureArray(b))
 }
diff --git a/src/react/QueryClientProvider.tsx b/src/react/QueryClientProvider.tsx
index 174363785..e0ea8baa5 100644
--- a/src/react/QueryClientProvider.tsx
+++ b/src/react/QueryClientProvider.tsx
@@ -9,16 +9,16 @@ declare global {
 }
 
 const defaultContext = React.createContext<QueryClient | undefined>(undefined)
+const QueryClientSharingContext = React.createContext<boolean>(false)
 
-// We share the first and at least one
+// if contextSharing is on, we share the first and at least one
 // instance of the context across the window
 // to ensure that if React Query is used across
 // different bundles or microfrontends they will
 // all use the same **instance** of context, regardless
 // of module scoping.
-function getQueryClientContext() {
-  // @ts-ignore (for global)
-  if (typeof window !== 'undefined') {
+function getQueryClientContext(contextSharing: boolean) {
+  if (contextSharing && typeof window !== 'undefined') {
     if (!window.ReactQueryClientContext) {
       window.ReactQueryClientContext = defaultContext
     }
@@ -30,7 +30,9 @@ function getQueryClientContext() {
 }
 
 export const useQueryClient = () => {
-  const queryClient = React.useContext(getQueryClientContext())
+  const queryClient = React.useContext(
+    getQueryClientContext(React.useContext(QueryClientSharingContext))
+  )
 
   if (!queryClient) {
     throw new Error('No QueryClient set, use QueryClientProvider to set one')
@@ -41,10 +43,12 @@ export const useQueryClient = () => {
 
 export interface QueryClientProviderProps {
   client: QueryClient
+  contextSharing?: boolean
 }
 
 export const QueryClientProvider: React.FC<QueryClientProviderProps> = ({
   client,
+  contextSharing = false,
   children,
 }) => {
   React.useEffect(() => {
@@ -54,7 +58,11 @@ export const QueryClientProvider: React.FC<QueryClientProviderProps> = ({
     }
   }, [client])
 
-  const Context = getQueryClientContext()
+  const Context = getQueryClientContext(contextSharing)
 
-  return <Context.Provider value={client}>{children}</Context.Provider>
+  return (
+    <QueryClientSharingContext.Provider value={contextSharing}>
+      <Context.Provider value={client}>{children}</Context.Provider>
+    </QueryClientSharingContext.Provider>
+  )
 }


Test patch:
diff --git a/src/core/tests/queryClient.test.tsx b/src/core/tests/queryClient.test.tsx
index f34d7a080..016324bf2 100644
--- a/src/core/tests/queryClient.test.tsx
+++ b/src/core/tests/queryClient.test.tsx
@@ -550,6 +550,22 @@ describe('queryClient', () => {
       expect(queryFn1).toHaveBeenCalledTimes(1)
       expect(queryFn2).toHaveBeenCalledTimes(1)
     })
+
+    test('should not refetch active queries when "refetchActive" is false', async () => {
+      const key1 = queryKey()
+      const queryFn1 = jest.fn()
+      await queryClient.fetchQuery(key1, queryFn1)
+      const observer = new QueryObserver(queryClient, {
+        queryKey: key1,
+        staleTime: Infinity,
+      })
+      const unsubscribe = observer.subscribe()
+      queryClient.invalidateQueries(key1, {
+        refetchActive: false,
+      })
+      unsubscribe()
+      expect(queryFn1).toHaveBeenCalledTimes(1)
+    })
   })
 
   describe('resetQueries', () => {
diff --git a/src/core/tests/queryObserver.test.tsx b/src/core/tests/queryObserver.test.tsx
index 35939d749..1467e50cb 100644
--- a/src/core/tests/queryObserver.test.tsx
+++ b/src/core/tests/queryObserver.test.tsx
@@ -240,6 +240,52 @@ describe('queryObserver', () => {
     expect(observerResult2.data).toMatchObject({ myCount: 1 })
   })
 
+  test('should always run the selector again if selector throws an error', async () => {
+    const key = queryKey()
+    const results: QueryObserverResult[] = []
+    const select = () => {
+      throw new Error('selector error')
+    }
+    const queryFn = () => ({ count: 1 })
+    const observer = new QueryObserver(queryClient, {
+      queryKey: key,
+      queryFn,
+      select,
+    })
+    const unsubscribe = observer.subscribe(result => {
+      results.push(result)
+    })
+    await sleep(1)
+    await observer.refetch()
+    unsubscribe()
+    expect(results.length).toBe(5)
+    expect(results[0]).toMatchObject({
+      status: 'loading',
+      isFetching: true,
+      data: undefined,
+    })
+    expect(results[1]).toMatchObject({
+      status: 'error',
+      isFetching: false,
+      data: undefined,
+    })
+    expect(results[2]).toMatchObject({
+      status: 'error',
+      isFetching: true,
+      data: undefined,
+    })
+    expect(results[3]).toMatchObject({
+      status: 'error',
+      isFetching: false,
+      data: undefined,
+    })
+    expect(results[4]).toMatchObject({
+      status: 'error',
+      isFetching: false,
+      data: undefined,
+    })
+  })
+
   test('should structurally share the selector', async () => {
     const key = queryKey()
     let count = 0