refactor(types): make queryKey required on ObserverOptions (#6879)

* refactor(types): make queryKey required on ObserverOptions we cannot create an observer without a key, because an observer, per definition, watches a certain query by its key * test: observers need a key * chore: remove a branch that can't exist we can't have an observer without a queryKey * fix: when comparing with shallowEqualObjects, the currentResult must come first because it doesn't detect length changes * test: we get another event * types: you can't set queryKey as default * a test for undefined queryKey makes no sense * fix: more type issues * types: queryKey is also not required for defaultOptions in solid * fix(core): shallowEqualObjects should return false for objects with different lengths as we use it for options, and the length of options can change
TanStack · Feb 17, 2024 · 61f4d74 · 61f4d74
1 parent 7595412
commit 61f4d74
Show file tree

Hide file tree

Showing 22 changed files with 127 additions and 141 deletions.
diff --git a/docs/reference/QueryCache.md b/docs/reference/QueryCache.md
@@ -63,7 +63,6 @@ const query = queryCache.find(queryKey)
 
 **Options**
 
-- `queryKey?: QueryKey`: [Query Keys](./framework/react/guides/query-keys)
 - `filters?: QueryFilters`: [Query Filters](./framework/react/guides/filters#query-filters)
 
 **Returns**

diff --git a/packages/angular-query-experimental/src/types.ts b/packages/angular-query-experimental/src/types.ts
@@ -12,7 +12,6 @@ import type {
   QueryKey,
   QueryObserverOptions,
   QueryObserverResult,
-  WithRequired,
 } from '@tanstack/query-core'
 import type { MapToSignals } from './signal-proxy'
 
@@ -22,9 +21,12 @@ export interface CreateBaseQueryOptions<
   TData = TQueryFnData,
   TQueryData = TQueryFnData,
   TQueryKey extends QueryKey = QueryKey,
-> extends WithRequired<
-    QueryObserverOptions<TQueryFnData, TError, TData, TQueryData, TQueryKey>,
-    'queryKey'
+> extends QueryObserverOptions<
+    TQueryFnData,
+    TError,
+    TData,
+    TQueryData,
+    TQueryKey
   > {}
 
 export interface CreateQueryOptions<
@@ -33,15 +35,12 @@ export interface CreateQueryOptions<
   TData = TQueryFnData,
   TQueryKey extends QueryKey = QueryKey,
 > extends Omit<
-    WithRequired<
-      CreateBaseQueryOptions<
-        TQueryFnData,
-        TError,
-        TData,
-        TQueryFnData,
-        TQueryKey
-      >,
-      'queryKey'
+    CreateBaseQueryOptions<
+      TQueryFnData,
+      TError,
+      TData,
+      TQueryFnData,
+      TQueryKey
     >,
     'suspense'
   > {}
@@ -83,19 +82,16 @@ export interface CreateInfiniteQueryOptions<
   TQueryData = TQueryFnData,
   TQueryKey extends QueryKey = QueryKey,
   TPageParam = unknown,
-> extends WithRequired<
-    Omit<
-      InfiniteQueryObserverOptions<
-        TQueryFnData,
-        TError,
-        TData,
-        TQueryData,
-        TQueryKey,
-        TPageParam
-      >,
-      'suspense'
+> extends Omit<
+    InfiniteQueryObserverOptions<
+      TQueryFnData,
+      TError,
+      TData,
+      TQueryData,
+      TQueryKey,
+      TPageParam
     >,
-    'queryKey'
+    'suspense'
   > {}
 
 export type CreateBaseQueryResult<

diff --git a/packages/query-core/src/infiniteQueryObserver.ts b/packages/query-core/src/infiniteQueryObserver.ts
@@ -70,7 +70,7 @@ export class InfiniteQueryObserver<
   }
 
   setOptions(
-    options?: InfiniteQueryObserverOptions<
+    options: InfiniteQueryObserverOptions<
       TQueryFnData,
       TError,
       TData,

diff --git a/packages/query-core/src/mutationObserver.ts b/packages/query-core/src/mutationObserver.ts
@@ -59,7 +59,7 @@ export class MutationObserver<
       | MutationObserverOptions<TData, TError, TVariables, TContext>
       | undefined
     this.options = this.#client.defaultMutationOptions(options)
-    if (!shallowEqualObjects(prevOptions, this.options)) {
+    if (!shallowEqualObjects(this.options, prevOptions)) {
       this.#client.getMutationCache().notify({
         type: 'observerOptionsUpdated',
         mutation: this.#currentMutation,

diff --git a/packages/query-core/src/queryCache.ts b/packages/query-core/src/queryCache.ts
@@ -99,10 +99,13 @@ export class QueryCache extends Subscribable<QueryCacheListener> {
 
   build<TQueryFnData, TError, TData, TQueryKey extends QueryKey>(
     client: QueryClient,
-    options: QueryOptions<TQueryFnData, TError, TData, TQueryKey>,
+    options: WithRequired<
+      QueryOptions<TQueryFnData, TError, TData, TQueryKey>,
+      'queryKey'
+    >,
     state?: QueryState<TData, TError>,
   ): Query<TQueryFnData, TError, TData, TQueryKey> {
-    const queryKey = options.queryKey!
+    const queryKey = options.queryKey
     const queryHash =
       options.queryHash ?? hashQueryKeyByOptions(queryKey, options)
     let query = this.get<TQueryFnData, TError, TData, TQueryKey>(queryHash)

diff --git a/packages/query-core/src/queryClient.ts b/packages/query-core/src/queryClient.ts
@@ -41,7 +41,7 @@ import type { MutationFilters, QueryFilters, Updater } from './utils'
 
 interface QueryDefaults {
   queryKey: QueryKey
-  defaultOptions: QueryOptions<any, any, any>
+  defaultOptions: Omit<QueryOptions<any, any, any>, 'queryKey'>
 }
 
 interface MutationDefaults {
@@ -409,10 +409,13 @@ export class QueryClient {
 
   getQueryDefaults(
     queryKey: QueryKey,
-  ): QueryObserverOptions<any, any, any, any, any> {
+  ): Omit<QueryObserverOptions<any, any, any, any, any>, 'queryKey'> {
     const defaults = [...this.#queryDefaults.values()]
 
-    let result: QueryObserverOptions<any, any, any, any, any> = {}
+    let result: Omit<
+      QueryObserverOptions<any, any, any, any, any>,
+      'queryKey'
+    > = {}
 
     defaults.forEach((queryDefault) => {
       if (partialMatchKey(queryKey, queryDefault.queryKey)) {
@@ -456,7 +459,7 @@ export class QueryClient {
     TQueryKey extends QueryKey = QueryKey,
     TPageParam = never,
   >(
-    options?:
+    options:
       | QueryObserverOptions<
           TQueryFnData,
           TError,
@@ -479,7 +482,7 @@ export class QueryClient {
     TQueryData,
     TQueryKey
   > {
-    if (options?._defaulted) {
+    if (options._defaulted) {
       return options as DefaultedQueryObserverOptions<
         TQueryFnData,
         TError,
@@ -491,7 +494,7 @@ export class QueryClient {
 
     const defaultedOptions = {
       ...this.#defaultOptions.queries,
-      ...(options?.queryKey && this.getQueryDefaults(options.queryKey)),
+      ...this.getQueryDefaults(options.queryKey),
       ...options,
       _defaulted: true,
     }

diff --git a/packages/query-core/src/queryObserver.ts b/packages/query-core/src/queryObserver.ts
@@ -132,7 +132,7 @@ export class QueryObserver<
   }
 
   setOptions(
-    options?: QueryObserverOptions<
+    options: QueryObserverOptions<
       TQueryFnData,
       TError,
       TData,
@@ -146,7 +146,7 @@ export class QueryObserver<
 
     this.options = this.#client.defaultQueryOptions(options)
 
-    if (!shallowEqualObjects(prevOptions, this.options)) {
+    if (!shallowEqualObjects(this.options, prevOptions)) {
       this.#client.getQueryCache().notify({
         type: 'observerOptionsUpdated',
         query: this.#currentQuery,
@@ -161,11 +161,6 @@ export class QueryObserver<
       throw new Error('Expected enabled to be a boolean')
     }
 
-    // Keep previous query key if the user does not supply one
-    if (!this.options.queryKey) {
-      this.options.queryKey = prevOptions.queryKey
-    }
-
     this.#updateQuery()
 
     const mounted = this.hasListeners()

diff --git a/packages/query-core/src/tests/queriesObserver.test.tsx b/packages/query-core/src/tests/queriesObserver.test.tsx
@@ -34,31 +34,6 @@ describe('queriesObserver', () => {
     expect(observerResult).toMatchObject([{ data: 1 }, { data: 2 }])
   })
 
-  test('should still return value for undefined query key', async () => {
-    const consoleMock = vi.spyOn(console, 'error')
-    consoleMock.mockImplementation(() => undefined)
-    const key1 = queryKey()
-    const queryFn1 = vi.fn().mockReturnValue(1)
-    const queryFn2 = vi.fn().mockReturnValue(2)
-    const observer = new QueriesObserver(queryClient, [
-      { queryKey: key1, queryFn: queryFn1 },
-      { queryKey: undefined, queryFn: queryFn2 },
-    ])
-    let observerResult
-    const unsubscribe = observer.subscribe((result) => {
-      observerResult = result
-    })
-    await sleep(1)
-    unsubscribe()
-    expect(observerResult).toMatchObject([{ data: 1 }, { data: 2 }])
-
-    expect(consoleMock).toHaveBeenCalledTimes(1)
-    expect(consoleMock).toHaveBeenCalledWith(
-      "As of v4, queryKey needs to be an Array. If you are using a string like 'repoData', please change it to an Array, e.g. ['repoData']",
-    )
-    consoleMock.mockRestore()
-  })
-
   test('should update when a query updates', async () => {
     const key1 = queryKey()
     const key2 = queryKey()

diff --git a/packages/query-core/src/tests/queryCache.test.tsx b/packages/query-core/src/tests/queryCache.test.tsx
@@ -53,10 +53,11 @@ describe('queryCache', () => {
       const unsubScribeObserver = observer.subscribe(vi.fn())
 
       await waitFor(() => {
-        expect(events.length).toBe(8)
+        expect(events.length).toBe(9)
       })
 
       expect(events).toEqual([
+        'observerOptionsUpdated',
         'added', // 1. Query added -> loading
         'observerResultsUpdated', // 2. Observer result updated -> loading
         'observerAdded', // 3. Observer added

diff --git a/packages/query-core/src/tests/queryObserver.test.tsx b/packages/query-core/src/tests/queryObserver.test.tsx
@@ -459,7 +459,7 @@ describe('queryObserver', () => {
     const unsubscribe = observer.subscribe((x) => {
       results.push(x)
     })
-    observer.setOptions({ enabled: false, staleTime: 10 })
+    observer.setOptions({ queryKey: key, enabled: false, staleTime: 10 })
     await queryClient.fetchQuery({ queryKey: key, queryFn })
     await sleep(100)
     unsubscribe()
@@ -579,7 +579,7 @@ describe('queryObserver', () => {
 
     const firstData = observer.getCurrentResult().data
 
-    observer.setOptions({ placeholderData: {} })
+    observer.setOptions({ queryKey: key, placeholderData: {} })
 
     const secondData = observer.getCurrentResult().data
 
@@ -885,7 +885,7 @@ describe('queryObserver', () => {
 
     const spy = vi.fn()
     const unsubscribe = queryClient.getQueryCache().subscribe(spy)
-    observer.setOptions({ enabled: false })
+    observer.setOptions({ queryKey: key, enabled: false })
 
     expect(spy).toHaveBeenCalledTimes(1)
     expect(spy).toHaveBeenCalledWith(

diff --git a/packages/query-core/src/tests/queryObserver.types.test.tsx b/packages/query-core/src/tests/queryObserver.types.test.tsx
@@ -31,6 +31,7 @@ describe('placeholderData', () => {
         }
 
         new QueryObserver<boolean, CustomError>(createQueryClient(), {
+          queryKey: ['key'],
           placeholderData: (_, previousQuery) => {
             const error = previousQuery?.state.error
 
@@ -49,6 +50,7 @@ describe('placeholderData', () => {
         type QueryData = typeof queryData
 
         new QueryObserver(createQueryClient(), {
+          queryKey: ['key'],
           queryFn: () => queryData,
           select: (data) => data.foo,
           placeholderData: (previousData) => {

diff --git a/packages/query-core/src/tests/utils.test.tsx b/packages/query-core/src/tests/utils.test.tsx
@@ -7,11 +7,29 @@ import {
   matchMutation,
   partialMatchKey,
   replaceEqualDeep,
+  shallowEqualObjects,
 } from '../utils'
 import { Mutation } from '../mutation'
 import { createQueryClient } from './utils'
 
 describe('core/utils', () => {
+  describe('shallowEqualObjects', () => {
+    it('should return `true` for shallow equal objects', () => {
+      expect(shallowEqualObjects({ a: 1 }, { a: 1 })).toEqual(true)
+    })
+
+    it('should return `false` for non shallow equal objects', () => {
+      expect(shallowEqualObjects({ a: 1 }, { a: 2 })).toEqual(false)
+    })
+
+    it('should return `false` if lengths are not equal', () => {
+      expect(shallowEqualObjects({ a: 1 }, { a: 1, b: 2 })).toEqual(false)
+    })
+
+    it('should return false if b is undefined', () => {
+      expect(shallowEqualObjects({ a: 1 }, undefined)).toEqual(false)
+    })
+  })
   describe('isPlainObject', () => {
     it('should return `true` for a plain object', () => {
       expect(isPlainObject({})).toEqual(true)

diff --git a/packages/query-core/src/types.ts b/packages/query-core/src/types.ts
@@ -218,12 +218,9 @@ export interface QueryObserverOptions<
   TQueryData = TQueryFnData,
   TQueryKey extends QueryKey = QueryKey,
   TPageParam = never,
-> extends QueryOptions<
-    TQueryFnData,
-    TError,
-    TQueryData,
-    TQueryKey,
-    TPageParam
+> extends WithRequired<
+    QueryOptions<TQueryFnData, TError, TQueryData, TQueryKey, TPageParam>,
+    'queryKey'
   > {
   /**
    * Set this to `false` to disable automatic refetching when the query mounts or changes query keys.
@@ -340,6 +337,11 @@ export interface QueryObserverOptions<
 export type WithRequired<TTarget, TKey extends keyof TTarget> = TTarget & {
   [_ in TKey]: {}
 }
+export type Optional<TTarget, TKey extends keyof TTarget> = Pick<
+  Partial<TTarget>,
+  TKey
+> &
+  Omit<TTarget, TKey>
 
 export type DefaultedQueryObserverOptions<
   TQueryFnData = unknown,
@@ -871,7 +873,7 @@ export interface QueryClientConfig {
 }
 
 export interface DefaultOptions<TError = DefaultError> {
-  queries?: Omit<QueryObserverOptions<unknown, TError>, 'suspense'>
+  queries?: Omit<QueryObserverOptions<unknown, TError>, 'suspense' | 'queryKey'>
   mutations?: MutationObserverOptions<unknown, TError, unknown, unknown>
 }
 

diff --git a/packages/query-core/src/utils.ts b/packages/query-core/src/utils.ts
@@ -167,7 +167,7 @@ export function matchMutation(
 
 export function hashQueryKeyByOptions<TQueryKey extends QueryKey = QueryKey>(
   queryKey: TQueryKey,
-  options?: QueryOptions<any, any, any, TQueryKey>,
+  options?: Pick<QueryOptions<any, any, any, any>, 'queryKeyHashFn'>,
 ): string {
   const hashFn = options?.queryKeyHashFn || hashKey
   return hashFn(queryKey)
@@ -257,10 +257,13 @@ export function replaceEqualDeep(a: any, b: any): any {
 }
 
 /**
- * Shallow compare objects. Only works with objects that always have the same properties.
+ * Shallow compare objects.
  */
-export function shallowEqualObjects<T>(a: T, b: T): boolean {
-  if ((a && !b) || (b && !a)) {
+export function shallowEqualObjects<T extends Record<string, any>>(
+  a: T,
+  b: T | undefined,
+): boolean {
+  if (!b || Object.keys(a).length !== Object.keys(b).length) {
     return false
   }
 

diff --git a/packages/query-persist-client-core/src/__tests__/persist.test.ts b/packages/query-persist-client-core/src/__tests__/persist.test.ts
@@ -46,7 +46,9 @@ describe('persistQueryClientSave', () => {
     const queryFn = vi.fn().mockReturnValue(1)
     const observer = new QueriesObserver(queryClient, [{ queryKey, queryFn }])
     const unsubscribeObserver = observer.subscribe(vi.fn())
-    observer.getObservers()[0]?.setOptions({ refetchOnWindowFocus: false })
+    observer
+      .getObservers()[0]
+      ?.setOptions({ queryKey, refetchOnWindowFocus: false })
     unsubscribeObserver()
 
     queryClient.setQueryData(queryKey, 2)