chore: eslint no-unnecessary-condition (#3344)

* chore(eslint): turn on no-unnecessary-condition rule

* chore(eslint): turn on no-unnecessary-condition rule

remove unnecessary optional chainings in tests

* chore(eslint): keep the matchMedia check

even though it can never be undefined, except in tests, and I couldn't figure out how to mock that properly

* chore(eslint): remove unnecessary checks in devtools

* chore(eslint): addEventListener should exist on window

if window is defined, which is checked by isServer

* chore(eslint): assign default object to options

instead of re-assigning it. In any case, the optional chaining is unnecessary

* chore(eslint): action.type should always be defined

* chore(eslint): keep the fallback for console

* chore(eslint): one rule always complains

so up-casting false to boolean

* chore(eslint): if we have a behaviour, we also have na onFetch

* chore(eslint): parseFilters always returns an object

as it falls back to an empty object internally, so the falsy check didn't do anything

* chore(eslint): upcast previous result to be potentially undefined

to make the optinal chains necessary

* fix issues after updating to alpha

.eslintrc

@@ -12,6 +12,7 @@
     "es6": true
   },
   "parserOptions": {
+    "project": "./tsconfig.json",
     "sourceType": "module"
   },
   "rules": {
@@ -21,6 +22,7 @@
     "@typescript-eslint/no-empty-interface": "off",
     "@typescript-eslint/no-explicit-any": "off",
     "@typescript-eslint/no-non-null-assertion": "off",
+    "@typescript-eslint/no-unnecessary-condition": "error",
     "@typescript-eslint/no-inferrable-types": [
       "error",
       {

src/broadcastQueryClient-experimental/index.ts

@@ -24,18 +24,15 @@ export function broadcastQueryClient({
   const queryCache = queryClient.getQueryCache()
 
   queryClient.getQueryCache().subscribe(queryEvent => {
-    if (transaction || !queryEvent?.query) {
+    if (transaction) {
       return
     }
 
     const {
       query: { queryHash, queryKey, state },
     } = queryEvent
 
-    if (
-      queryEvent.type === 'updated' &&
-      queryEvent.action?.type === 'success'
-    ) {
+    if (queryEvent.type === 'updated' && queryEvent.action.type === 'success') {
       channel.postMessage({
         type: 'updated',
         queryHash,

src/core/focusManager.ts

@@ -14,7 +14,7 @@ export class FocusManager extends Subscribable {
   constructor() {
     super()
     this.setup = onFocus => {
-      if (!isServer && window?.addEventListener) {
+      if (!isServer) {
         const listener = () => onFocus()
         // Listen to visibillitychange and focus
         window.addEventListener('visibilitychange', listener, false)

src/core/hydration.ts

@@ -75,14 +75,12 @@ function defaultShouldDehydrateQuery(query: Query) {
 
 export function dehydrate(
   client: QueryClient,
-  options?: DehydrateOptions
+  options: DehydrateOptions = {}
 ): DehydratedState {
-  options = options || {}
-
   const mutations: DehydratedMutation[] = []
   const queries: DehydratedQuery[] = []
 
-  if (options?.dehydrateMutations !== false) {
+  if (options.dehydrateMutations !== false) {
     const shouldDehydrateMutation =
       options.shouldDehydrateMutation || defaultShouldDehydrateMutation
 
@@ -96,7 +94,7 @@ export function dehydrate(
       })
   }
 
-  if (options?.dehydrateQueries !== false) {
+  if (options.dehydrateQueries !== false) {
     const shouldDehydrateQuery =
       options.shouldDehydrateQuery || defaultShouldDehydrateQuery
 
@@ -125,7 +123,9 @@ export function hydrate(
   const mutationCache = client.getMutationCache()
   const queryCache = client.getQueryCache()
 
+  // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
   const mutations = (dehydratedState as DehydratedState).mutations || []
+  // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
   const queries = (dehydratedState as DehydratedState).queries || []
 
   mutations.forEach(dehydratedMutation => {

src/core/onlineManager.ts

@@ -14,7 +14,7 @@ export class OnlineManager extends Subscribable {
   constructor() {
     super()
     this.setup = onOnline => {
-      if (!isServer && window?.addEventListener) {
+      if (!isServer) {
         const listener = () => onOnline()
         // Listen to online
         window.addEventListener('online', listener, false)

src/core/query.ts

@@ -420,9 +420,7 @@ export class Query<
 
     addSignalProperty(context)
 
-    if (this.options.behavior?.onFetch) {
-      this.options.behavior?.onFetch(context)
-    }
+    this.options.behavior?.onFetch(context)
 
     // Store state in case the current fetch needs to be reverted
     this.revertState = this.state
@@ -463,7 +461,7 @@ export class Query<
     // Try to fetch the data
     this.retryer = createRetryer({
       fn: context.fetchFn as () => TData,
-      abort: abortController?.abort?.bind(abortController),
+      abort: abortController?.abort.bind(abortController),
       onSuccess: data => {
         if (typeof data === 'undefined') {
           onError(new Error('Query data cannot be undefined') as any)

src/core/queryClient.ts

@@ -267,12 +267,12 @@ export class QueryClient {
         query.invalidate()
       })
 
-      if (filters?.refetchType === 'none') {
+      if (filters.refetchType === 'none') {
         return Promise.resolve()
       }
       const refetchFilters: RefetchQueryFilters = {
         ...filters,
-        type: filters?.refetchType ?? filters?.type ?? 'active',
+        type: filters.refetchType ?? filters.type ?? 'active',
       }
       return this.refetchQueries(refetchFilters, options)
     })
@@ -302,7 +302,7 @@ export class QueryClient {
           query.fetch(undefined, {
             ...options,
             cancelRefetch: options?.cancelRefetch ?? true,
-            meta: { refetchPage: filters?.refetchPage },
+            meta: { refetchPage: filters.refetchPage },
           })
         )
     )

src/core/queryObserver.ts

@@ -399,7 +399,9 @@ export class QueryObserver<
   ): QueryObserverResult<TData, TError> {
     const prevQuery = this.currentQuery
     const prevOptions = this.options
-    const prevResult = this.currentResult
+    const prevResult = this.currentResult as
+      | QueryObserverResult<TData, TError>
+      | undefined
     const prevResultState = this.currentResultState
     const prevResultOptions = this.currentResultOptions
     const queryChange = query !== prevQuery
@@ -622,7 +624,9 @@ export class QueryObserver<
       return
     }
 
-    const prevQuery = this.currentQuery
+    const prevQuery = this.currentQuery as
+      | Query<TQueryFnData, TError, TQueryData, TQueryKey>
+      | undefined
     this.currentQuery = query
     this.currentQueryInitialState = query.state
     this.previousQueryResult = this.currentResult

src/core/tests/focusManager.test.tsx

@@ -1,9 +1,11 @@
 import { sleep } from '../utils'
 import { FocusManager } from '../focusManager'
+import { setIsServer } from '../../reactjs/tests/utils'
 
 describe('focusManager', () => {
   let focusManager: FocusManager
   beforeEach(() => {
+    jest.resetModules()
     focusManager = new FocusManager()
   })
 
@@ -61,17 +63,14 @@ describe('focusManager', () => {
     globalThis.document = document
   })
 
-  test('cleanup should still be undefined if window.addEventListener is not defined', async () => {
-    const { addEventListener } = globalThis.window
-
-    // @ts-expect-error
-    globalThis.window.addEventListener = undefined
+  test('cleanup should still be undefined if window is not defined', async () => {
+    const restoreIsServer = setIsServer(true)
 
     const unsubscribe = focusManager.subscribe(() => undefined)
     expect(focusManager['cleanup']).toBeUndefined()
 
     unsubscribe()
-    globalThis.window.addEventListener = addEventListener
+    restoreIsServer()
   })
 
   it('should replace default window listener when a new event listener is set', async () => {

src/core/tests/hydration.test.tsx

@@ -196,8 +196,8 @@ describe('dehydration and rehydration', () => {
 
     // This is testing implementation details that can change and are not
     // part of the public API, but is important for keeping the payload small
-    const dehydratedQuery = dehydrated?.queries.find(
-      query => query?.queryKey[0] === 'string'
+    const dehydratedQuery = dehydrated.queries.find(
+      query => query.queryKey[0] === 'string'
     )
     expect(dehydratedQuery).toBeUndefined()
 

src/core/tests/mutationCache.test.tsx

@@ -193,14 +193,14 @@ describe('mutationCache', () => {
 
       // we currently have no way to add multiple observers to the same mutation
       const currentMutation = observer1['currentMutation']!
-      currentMutation?.addObserver(observer1)
-      currentMutation?.addObserver(observer2)
+      currentMutation.addObserver(observer1)
+      currentMutation.addObserver(observer2)
 
       expect(currentMutation['observers'].length).toEqual(2)
       expect(queryClient.getMutationCache().getAll()).toHaveLength(1)
 
-      currentMutation?.removeObserver(observer1)
-      currentMutation?.removeObserver(observer2)
+      currentMutation.removeObserver(observer1)
+      currentMutation.removeObserver(observer2)
       expect(currentMutation['observers'].length).toEqual(0)
       expect(queryClient.getMutationCache().getAll()).toHaveLength(1)
       // wait for cacheTime to gc

src/core/tests/mutations.test.tsx

@@ -341,7 +341,7 @@ describe('mutations', () => {
     // because no use case has been found
     const currentMutation = mutation['currentMutation']!
     expect(currentMutation['observers'].length).toEqual(1)
-    currentMutation?.addObserver(mutation)
+    currentMutation.addObserver(mutation)
 
     expect(currentMutation['observers'].length).toEqual(1)
   })

src/core/tests/onlineManager.test.tsx

@@ -1,5 +1,6 @@
 import { OnlineManager } from '../onlineManager'
 import { sleep } from '../utils'
+import { setIsServer } from '../../reactjs/tests/utils'
 
 describe('onlineManager', () => {
   let onlineManager: OnlineManager
@@ -56,17 +57,14 @@ describe('onlineManager', () => {
     expect(remove2Spy).not.toHaveBeenCalled()
   })
 
-  test('cleanup should still be undefined if window.addEventListener is not defined', async () => {
-    const { addEventListener } = globalThis.window
-
-    // @ts-expect-error
-    globalThis.window.addEventListener = undefined
+  test('cleanup should still be undefined if window is not defined', async () => {
+    const restoreIsServer = setIsServer(true)
 
     const unsubscribe = onlineManager.subscribe(() => undefined)
     expect(onlineManager['cleanup']).toBeUndefined()
 
     unsubscribe()
-    globalThis.window.addEventListener = addEventListener
+    restoreIsServer()
   })
 
   test('it should replace default window listener when a new event listener is set', async () => {

src/core/tests/queryObserver.test.tsx

@@ -619,7 +619,7 @@ describe('queryObserver', () => {
       queryKey: key,
       queryFn: () => data,
       select: () => {
-        if (true) return selectedData1
+        return selectedData1
       },
       placeholderData: placeholderData1,
     })
@@ -655,7 +655,7 @@ describe('queryObserver', () => {
       queryKey: key,
       queryFn: () => data,
       select: () => {
-        if (true) return selectedData
+        return selectedData
       },
       placeholderData: placeholderData1,
     })

src/devtools/devtools.tsx

@@ -122,7 +122,7 @@ export function ReactQueryDevtools({
 
     const run = (moveEvent: MouseEvent) => {
       const delta = dragInfo.pageY - moveEvent.pageY
-      const newHeight = dragInfo?.originalHeight + delta
+      const newHeight = dragInfo.originalHeight + delta
 
       setDevtoolsHeight(newHeight)
 
@@ -153,13 +153,13 @@ export function ReactQueryDevtools({
     const ref = panelRef.current
     if (ref) {
       const handlePanelTransitionStart = () => {
-        if (ref && isResolvedOpen) {
+        if (isResolvedOpen) {
           ref.style.visibility = 'visible'
         }
       }
 
       const handlePanelTransitionEnd = () => {
-        if (ref && !isResolvedOpen) {
+        if (!isResolvedOpen) {
           ref.style.visibility = 'hidden'
         }
       }
@@ -925,7 +925,7 @@ export const ReactQueryDevtoolsPanel = React.forwardRef<
             >
               <Explorer
                 label="Data"
-                value={activeQuery?.state?.data}
+                value={activeQuery.state.data}
                 defaultExpanded={{}}
               />
             </div>

src/reactjs/tests/ssr-hydration.test.tsx

@@ -9,15 +9,7 @@ import {
   dehydrate,
   hydrate,
 } from '../..'
-import * as utils from '../../core/utils'
-import { createQueryClient, mockLogger, sleep } from './utils'
-
-// This monkey-patches the isServer-value from utils,
-// so that we can pretend to be in a server environment
-function setIsServer(isServer: boolean) {
-  // @ts-ignore
-  utils.isServer = isServer
-}
+import { createQueryClient, mockLogger, setIsServer, sleep } from './utils'
 
 async function fetchData<TData>(value: TData, ms?: number): Promise<TData> {
   await sleep(ms || 1)

src/reactjs/tests/ssr.test.tsx

@@ -134,7 +134,7 @@ describe('Server Side Rendering', () => {
       const query = useInfiniteQuery(key, queryFn)
       return (
         <ul>
-          {query.data?.pages?.map(page => (
+          {query.data?.pages.map(page => (
             <li key={page}>{page}</li>
           ))}
         </ul>

src/reactjs/tests/suspense.test.tsx

@@ -877,8 +877,6 @@ describe("useQuery's in Suspense mode", () => {
   })
 
   it('should error catched in error boundary without infinite loop when enabled changed', async () => {
-    const succeed = false
-
     function Page() {
       const queryKeys = '1'
       const [enabled, setEnabled] = React.useState(false)
@@ -887,11 +885,7 @@ describe("useQuery's in Suspense mode", () => {
         [queryKeys],
         async () => {
           await sleep(10)
-          if (!succeed) {
-            throw new Error('Suspense Error Bingo')
-          } else {
-            return 'data'
-          }
+          throw new Error('Suspense Error Bingo')
         },
         {
           retry: false,

src/reactjs/tests/utils.tsx

@@ -7,6 +7,7 @@ import {
   QueryClientConfig,
   QueryClientProvider,
 } from '../..'
+import * as utils from '../../core/utils'
 
 export function createQueryClient(config?: QueryClientConfig): QueryClient {
   jest.spyOn(console, 'error').mockImplementation(() => undefined)
@@ -94,3 +95,16 @@ export const executeMutation = (
 ): Promise<unknown> => {
   return queryClient.getMutationCache().build(queryClient, options).execute()
 }
+
+// This monkey-patches the isServer-value from utils,
+// so that we can pretend to be in a server environment
+export function setIsServer(isServer: boolean) {
+  const original = utils.isServer
+  // @ts-ignore
+  utils.isServer = isServer
+
+  return () => {
+    // @ts-ignore
+    utils.isServer = original
+  }
+}