ronantakizawa/codereview-bench huggingface dataset:
Repository: tannerlinsley/react-query
File: src/queryCache.js (JavaScript)

Code before:
    const query = {
      ...options,
      instances: [],
      state: reducer(undefined, {
        type: actionInit,
        initialStatus,
        initialData,
        hasInitialData,
        isStale,
        manual,
      }),
    }

    const dispatch = action => {
      query.state = reducer(query.state, action)
      query.instances.forEach(d => d.onStateUpdate(query.state))
      notifyGlobalListeners()
    }

    query.scheduleStaleTimeout = () => {
      if (query.config.staleTime === Infinity) {
        return
      }
      query.staleTimeout = setTimeout(() => {
        if (query) {
          dispatch({ type: actionMarkStale })
        }
      }, query.config.staleTime)
    }

    query.scheduleGarbageCollection = () => {
      query.cacheTimeout = setTimeout(
        () => {
          cache.removeQueries(d => d.queryHash === query.queryHash)
        },
        typeof query.state.data === 'undefined' &&
          query.state.status !== 'error'
          ? 0
          : query.config.cacheTime
      )
    }

    query.heal = () => {
      // Stop the query from being garbage collected
      clearTimeout(query.cacheTimeout)

      // Mark the query as not cancelled
      query.cancelled = null
    }

    query.subscribe = instance => {

Review comment (refactor):
I couldn't see anywhere in the code where `query` would be `falsy` and especially now that we're clearing the timeout when they're removed I think we're good to simplify this.

Code after:

    const query = {
      ...options,
      instances: [],
      state: reducer(undefined, {
        type: actionInit,
        initialStatus,
        initialData,
        hasInitialData,
        isStale,
        manual,
      }),
    }

    const dispatch = action => {
      query.state = reducer(query.state, action)
      query.instances.forEach(d => d.onStateUpdate(query.state))
      notifyGlobalListeners()
    }

    query.scheduleStaleTimeout = () => {
      if (query.config.staleTime === Infinity) {
        return
      }
      query.staleTimeout = setTimeout(() => {
        dispatch({ type: actionMarkStale })
      }, query.config.staleTime)
    }

    query.scheduleGarbageCollection = () => {
      query.cacheTimeout = setTimeout(
        () => {
          cache.removeQueries(d => d.queryHash === query.queryHash)
        },
        typeof query.state.data === 'undefined' &&
          query.state.status !== 'error'
          ? 0
          : query.config.cacheTime
      )
    }

    query.heal = () => {
      // Stop the query from being garbage collected
      clearTimeout(query.cacheTimeout)

      // Mark the query as not cancelled
      query.cancelled = null
    }

    query.subscribe = instance => {
      let found = query.instances.find(d => d.id === instance.id)
Repository: tannerlinsley/react-query
File: src/tests/useQuery.test.js (JavaScript)

Code before:

    const { getByTestId } = render(<Page />)

    await waitForElement(() => getByTestId('status'))
    act(() => {
      expect(getByTestId('status').textContent).toBe('loading')
    })
  })

  // See https://github.com/tannerlinsley/react-query/issues/144
  it('should be in "success" state by default in manual mode', async () => {
    function Page() {
      const { status } = useQuery(
        'test',
        async () => {
          await sleep(10)
          return 'test'
        },
        {
          manual: true,
        }
      )

      return (
        <div>
          <h1>{status}</h1>
        </div>
      )
    }

    const rendered = render(<Page />)

    rendered.getByText('success')
  })

  // See https://github.com/tannerlinsley/react-query/issues/147
  it('should not pass stringified variables to query function', async () => {
    const queryFn = jest.fn()
    queryFn.mockImplementation(() => sleep(10))

    const variables = { number: 5, boolean: false, object: {}, array: [] }

    function Page() {
      useQuery(['test', variables], queryFn)

      return null
    }

    render(<Page />)

    expect(queryFn).toHaveBeenCalledWith('test', variables)

Review comment (bug):
Suspicion: due to GC happening on subsequent tests that was fixed in #332, this may have been passing as a fluke. The test before this ensures it's in a `loading` state by default, which is what the rendered output looks like for this test when it was failing.

Code after:
      expect(getByTestId('status').textContent).toBe('loading')
    })
  })

  // See https://github.com/tannerlinsley/react-query/issues/144
  it('should be in "success" state by default in manual mode', async () => {
    function Page() {
      const { status } = useQuery(
        'test',
        async () => {
          await sleep(10)
          return 'test'
        },
        {
          manual: true,
        }
      )

      return (
        <div>
          <h1>{status}</h1>
        </div>
      )
    }

    const { findByText } = render(<Page />)

    await findByText('success')
  })

  // See https://github.com/tannerlinsley/react-query/issues/147
  it('should not pass stringified variables to query function', async () => {
    const queryFn = jest.fn()
    queryFn.mockImplementation(() => sleep(10))

    const variables = { number: 5, boolean: false, object: {}, array: [] }

    function Page() {
      useQuery(['test', variables], queryFn)

      return null
    }

    render(<Page />)

    expect(queryFn).toHaveBeenCalledWith('test', variables)
  })

  // See https://github.com/tannerlinsley/react-query/issues/161
  it('should not fetch query when `manual` is set to `true`', async () => {
    const queryFn = jest.fn()
Repository: tannerlinsley/react-query
File: packages/query-core/src/types.ts (TypeScript)

Code before:
  TPageParam = never,
> extends WithRequired<
    QueryOptions<TQueryFnData, TError, TData, TQueryKey, TPageParam>,
    'queryKey'
  > {
  /**
   * The time in milliseconds after data is considered stale.
   * If the data is fresh it will be returned from the cache.
   */
  staleTime?: number
}

export interface EnsureQueryDataOptions<
  TQueryFnData = unknown,
  TError = DefaultError,
  TData = TQueryFnData,
  TQueryKey extends QueryKey = QueryKey,
  TPageParam = never,
  > extends FetchQueryOptions<
    TQueryFnData,
    TError,
    TData,
    TQueryKey,
    TPageParam
  > {
  revalidateIfStale?: boolean
}


type FetchInfiniteQueryPages<TQueryFnData = unknown, TPageParam = unknown> =
  | { pages?: never }
  | {
      pages: number
      getNextPageParam: GetNextPageParamFunction<TPageParam, TQueryFnData>
    }

export type FetchInfiniteQueryOptions<
  TQueryFnData = unknown,
  TError = DefaultError,
  TData = TQueryFnData,
  TQueryKey extends QueryKey = QueryKey,
  TPageParam = unknown,
> = FetchQueryOptions<
  TQueryFnData,
  TError,
  InfiniteData<TData, TPageParam>,
  TQueryKey,
  TPageParam
> &
  InitialPageParam<TPageParam> &
  FetchInfiniteQueryPages<TQueryFnData, TPageParam>

Review comment (suggestion):
going back and forth on my own proposal: since we don't use the word `revalidate` anywhere else, maybe it should be `refetchIfStale` ? Like `refetchOnMount` etc ... I think I like `revalidate` better, because it points towards it happening in the background, but I'm not sure if we should introduce new terminology 🤔 

Code after:
  TPageParam = never,
> extends WithRequired<
    QueryOptions<TQueryFnData, TError, TData, TQueryKey, TPageParam>,
    'queryKey'
  > {
  /**
   * The time in milliseconds after data is considered stale.
   * If the data is fresh it will be returned from the cache.
   */
  staleTime?: number
}

export interface EnsureQueryDataOptions<
  TQueryFnData = unknown,
  TError = DefaultError,
  TData = TQueryFnData,
  TQueryKey extends QueryKey = QueryKey,
  TPageParam = never,
> extends FetchQueryOptions<
    TQueryFnData,
    TError,
    TData,
    TQueryKey,
    TPageParam
  > {
  revalidateIfStale?: boolean
}

type FetchInfiniteQueryPages<TQueryFnData = unknown, TPageParam = unknown> =
  | { pages?: never }
  | {
      pages: number
      getNextPageParam: GetNextPageParamFunction<TPageParam, TQueryFnData>
    }

export type FetchInfiniteQueryOptions<
  TQueryFnData = unknown,
  TError = DefaultError,
  TData = TQueryFnData,
  TQueryKey extends QueryKey = QueryKey,
  TPageParam = unknown,
> = FetchQueryOptions<
  TQueryFnData,
  TError,
  InfiniteData<TData, TPageParam>,
  TQueryKey,
  TPageParam
> &
  InitialPageParam<TPageParam> &
  FetchInfiniteQueryPages<TQueryFnData, TPageParam>
Repository: tannerlinsley/react-query
File: packages/angular-query-experimental/src/inject-query-client.ts (TypeScript)

Code before:
import { Injector, inject } from '@angular/core'
import { QueryClient } from '@tanstack/query-core'
import type { InjectOptions } from '@angular/core'
import type { WithOptionalInjector } from './types'

/**
 * Injects a `QueryClient` instance and allows passing a custom injector.
 * @param injectOptions - Type of the options argument to inject and optionally a custom injector.
 * @returns The `QueryClient` instance.
 * @public
 * @deprecated Use `inject(QueryClient)` instead.
 * If you need to get a `QueryClient` from a custom injector, use `injector.get(QueryClient)`.
 *
 *
 * **Example**
 * ```ts
 * const queryClient = injectQueryClient();
 * ```
 */
export function injectQueryClient(
  injectOptions: InjectOptions & WithOptionalInjector = {},
) {
  return (injectOptions.injector ?? inject(Injector)).get(QueryClient)
}


Review comment (suggestion):
No need to change `injectQueryClient` as it's deprecated and will be removed.

Code after:
import { Injector, inject } from '@angular/core'
import { QueryClient } from '@tanstack/query-core'
import type { InjectOptions } from '@angular/core'

/**
 * Injects a `QueryClient` instance and allows passing a custom injector.
 * @param injectOptions - Type of the options argument to inject and optionally a custom injector.
 * @returns The `QueryClient` instance.
 * @public
 * @deprecated Use `inject(QueryClient)` instead.
 * If you need to get a `QueryClient` from a custom injector, use `injector.get(QueryClient)`.
 *
 *
 * **Example**
 * ```ts
 * const queryClient = injectQueryClient();
 * ```
 */
export function injectQueryClient(
  injectOptions: InjectOptions & { injector?: Injector } = {},
) {
  return (injectOptions.injector ?? inject(Injector)).get(QueryClient)
}
Repository: tannerlinsley/react-query
File: packages/query-core/src/query.ts (TypeScript)

Code before:

interface SuccessAction<TData> {
  data: TData | undefined
  type: 'success'
  dataUpdatedAt?: number
  manual?: boolean
}

interface ErrorAction<TError> {
  type: 'error'
  error: TError
}

interface InvalidateAction {
  type: 'invalidate'
}

interface PauseAction {
  type: 'pause'
}

interface ContinueAction {
  type: 'continue'
}

export interface RefetchActionEvent {
  type: 'ACTION-REFETCH'
}

export interface InvalidateActionEvent {
  type: 'ACTION-INVALIDATE'
}

export interface TriggerErrorActionEvent {
  type: 'ACTION-TRIGGER-ERROR'
}

export interface RestoreErrorActionEvent {
  type: 'ACTION-RESTORE-ERROR'
}

export interface ResetActionEvent {
  type: 'ACTION-RESET'
}

export interface RemoveActionEvent {
  type: 'ACTION-REMOVE'
}

export interface TriggerLoadingActionEvent {
  type: 'ACTION-TRIGGER-LOADING'

Review comment (performance):
I don’t think these should be here. The actions listened on the NotifyEvents are events that are dispatched from our query reducer. Consumers can listen to those to observe updates that happen in the cache. When the devtools trigger an update that will result in an internal state change, those changes will automatically be propagated.

Can you explain why those were added?

Code after:

interface SuccessAction<TData> {
  data: TData | undefined
  type: 'success'
  dataUpdatedAt?: number
  manual?: boolean
}

interface ErrorAction<TError> {
  type: 'error'
  error: TError
}

interface InvalidateAction {
  type: 'invalidate'
}

interface PauseAction {
  type: 'pause'
}

interface ContinueAction {
  type: 'continue'
}

interface SetStateAction<TData, TError> {
  type: 'setState'
  state: Partial<QueryState<TData, TError>>
  setStateOptions?: SetStateOptions
}

export type Action<TData, TError> =
  | ContinueAction
  | ErrorAction<TError>
  | FailedAction<TError>
  | FetchAction
  | InvalidateAction
  | PauseAction
  | SetStateAction<TData, TError>
  | SuccessAction<TData>

export interface SetStateOptions {
  meta?: any
}

// CLASS

export class Query<
  TQueryFnData = unknown,
  TError = DefaultError,
  TData = TQueryFnData,
Repository: tannerlinsley/react-query
File: packages/query-devtools/src/Devtools.tsx (TypeScript)

Code before:
import { usePiPWindow, useQueryDevtoolsContext, useTheme } from './contexts'
import {
  BUTTON_POSITION,
  DEFAULT_HEIGHT,
  DEFAULT_MUTATION_SORT_FN_NAME,
  DEFAULT_SORT_FN_NAME,
  DEFAULT_SORT_ORDER,
  DEFAULT_WIDTH,
  INITIAL_IS_OPEN,
  POSITION,
  firstBreakpoint,
  secondBreakpoint,
  thirdBreakpoint,
} from './constants'
import type {
  DevtoolsErrorType,
  DevtoolsPosition,
  QueryDevtoolsProps,
} from './contexts'
import type {
  Mutation,
  MutationCache,
  Query,
  QueryCache,
  QueryCacheNotifyEvent,
  QueryClient,
  QueryState,
} from '@tanstack/query-core'
import type { StorageObject, StorageSetter } from '@solid-primitives/storage'
import type { Accessor, Component, JSX, Setter } from 'solid-js'

interface DevtoolsPanelProps {
  localStore: StorageObject<string>
  setLocalStore: StorageSetter<string, unknown>
}

interface ContentViewProps {
  localStore: StorageObject<string>
  setLocalStore: StorageSetter<string, unknown>
  showPanelViewOnly?: boolean
  onClose?: () => unknown
}

interface QueryStatusProps {
  label: string
  color: 'green' | 'yellow' | 'gray' | 'blue' | 'purple' | 'red'
  count: number
}

const [selectedQueryHash, setSelectedQueryHash] = createSignal<string | null>(
  null,

Review comment (bug):
according to the failing build, this import is unused

```suggestion
```

Code after:
import { usePiPWindow, useQueryDevtoolsContext, useTheme } from './contexts'
import {
  BUTTON_POSITION,
  DEFAULT_HEIGHT,
  DEFAULT_MUTATION_SORT_FN_NAME,
  DEFAULT_SORT_FN_NAME,
  DEFAULT_SORT_ORDER,
  DEFAULT_WIDTH,
  INITIAL_IS_OPEN,
  POSITION,
  firstBreakpoint,
  secondBreakpoint,
  thirdBreakpoint,
} from './constants'
import type {
  DevtoolsErrorType,
  DevtoolsPosition,
  QueryDevtoolsProps,
} from './contexts'
import type {
  Mutation,
  MutationCache,
  Query,
  QueryCache,
  QueryCacheNotifyEvent,
  QueryState,
} from '@tanstack/query-core'
import type { StorageObject, StorageSetter } from '@solid-primitives/storage'
import type { Accessor, Component, JSX, Setter } from 'solid-js'

interface DevtoolsPanelProps {
  localStore: StorageObject<string>
  setLocalStore: StorageSetter<string, unknown>
}

interface ContentViewProps {
  localStore: StorageObject<string>
  setLocalStore: StorageSetter<string, unknown>
  showPanelViewOnly?: boolean
  onClose?: () => unknown
}

interface QueryStatusProps {
  label: string
  color: 'green' | 'yellow' | 'gray' | 'blue' | 'purple' | 'red'
  count: number
}

const [selectedQueryHash, setSelectedQueryHash] = createSignal<string | null>(
  null,
)
Repository: tannerlinsley/react-query
File: packages/query-devtools/src/Devtools.tsx (TypeScript)

Code before:
  createEffect(() => {
    setValue(callback(mutationCache))
  })

  mutationCacheMap.set(callback, setValue)

  onCleanup(() => {
    mutationCacheMap.delete(callback)
  })

  return value
}

export type DevToolsActionType =
  | 'REFETCH'
  | 'INVALIDATE'
  | 'RESET'
  | 'REMOVE'
  | 'TRIGGER_ERROR'
  | 'RESTORE_ERROR'
  | 'TRIGGER_LOADING'
  | 'RESTORE_LOADING'
  | 'CLEAR_MUTATION_CACHE'
  | 'CLEAR_QUERY_CACHE'

export const DEV_TOOLS_EVENT = '@tanstack/query-devtools-event'

export const sendDevToolsEvent = ({
  type,
  queryHash,
  metadata,
}: {
  type: DevToolsActionType
  queryHash?: string
  metadata?: Record<string, unknown>
}) => {
  const event = new CustomEvent(DEV_TOOLS_EVENT, {
    detail: { type, queryHash, metadata },
    bubbles: true,
    cancelable: true,
  })
  window.dispatchEvent(event)
}

const stylesFactory = (
  theme: 'light' | 'dark',
  css: (typeof goober)['css'],
) => {
  const { colors, font, size, alpha, shadow, border } = tokens

  const t = (light: string, dark: string) => (theme === 'light' ? light : dark)

Review comment (suggestion):
```suggestion
const DEV_TOOLS_EVENT = '@tanstack/query-devtools-event'
```

Code after:
  createEffect(() => {
    setValue(callback(mutationCache))
  })

  mutationCacheMap.set(callback, setValue)

  onCleanup(() => {
    mutationCacheMap.delete(callback)
  })

  return value
}

type DevToolsActionType =
  | 'REFETCH'
  | 'INVALIDATE'
  | 'RESET'
  | 'REMOVE'
  | 'TRIGGER_ERROR'
  | 'RESTORE_ERROR'
  | 'TRIGGER_LOADING'
  | 'RESTORE_LOADING'
  | 'CLEAR_MUTATION_CACHE'
  | 'CLEAR_QUERY_CACHE'

const DEV_TOOLS_EVENT = '@tanstack/query-devtools-event'

const sendDevToolsEvent = ({
  type,
  queryHash,
  metadata,
}: {
  type: DevToolsActionType
  queryHash?: string
  metadata?: Record<string, unknown>
}) => {
  const event = new CustomEvent(DEV_TOOLS_EVENT, {
    detail: { type, queryHash, metadata },
    bubbles: true,
    cancelable: true,
  })
  window.dispatchEvent(event)
}

const stylesFactory = (
  theme: 'light' | 'dark',
  css: (typeof goober)['css'],
) => {
  const { colors, font, size, alpha, shadow, border } = tokens

  const t = (light: string, dark: string) => (theme === 'light' ? light : dark)
Repository: tannerlinsley/react-query
File: packages/query-core/src/__tests__/hydration.test.tsx (TypeScript)

Code before:
          shouldDehydrateQuery: () => true,
          shouldRedactErrors: () => false,
        },
      },
    })

    const testError = new Error('original error')

    const promise = queryClient
      .prefetchQuery({
        queryKey: ['error'],
        queryFn: () => Promise.reject(testError),
        retry: false,
      })
      .catch(() => undefined)

    const dehydrated = dehydrate(queryClient)

    expect(dehydrated.queries[0]?.promise).toBeInstanceOf(Promise)

    try {
      await dehydrated.queries[0]?.promise
      expect(true).toBe(false)
    } catch (error) {
      expect(error).toBe(testError)
    }

    await promise
    queryClient.clear()
  })

  test('should handle errors in promises for pending queries', async () => {
    const consoleMock = vi.spyOn(console, 'error')
    consoleMock.mockImplementation(() => undefined)

    const originalNodeEnv = process.env.NODE_ENV
    process.env.NODE_ENV = 'production'

    const queryCache = new QueryCache()
    const queryClient = createQueryClient({
      queryCache,
      defaultOptions: {
        dehydrate: {
          shouldDehydrateQuery: () => true,
        },
      },
    })

    const promise = queryClient
      .prefetchQuery({
        queryKey: ['error'],

Review comment (suggestion):
I think this should be:

```
await expect(dehydrated.queries[0]?.promise).rejects.toBe(testError)
```


Code after:

    const promise = serverQueryClient.prefetchQuery(query)

    let dehydrated = dehydrate(serverQueryClient)

    // --- client ---

    const clientQueryClient = createQueryClient({
      defaultOptions: {
        hydrate: {
          deserializeData: deserializeDataMock,
        },
      },
    })

    hydrate(clientQueryClient, dehydrated)

    await vi.waitFor(() => promise)
    await vi.waitFor(() =>
      expect(clientQueryClient.getQueryData(query.queryKey)).toBe(0),
    )

    expect(serializeDataMock).toHaveBeenCalledTimes(1)
    expect(serializeDataMock).toHaveBeenCalledWith(0)

    expect(deserializeDataMock).toHaveBeenCalledTimes(1)
    expect(deserializeDataMock).toHaveBeenCalledWith(0)

    // --- server ---
    countRef.current++
    serverQueryClient.clear()
    const promise2 = serverQueryClient.prefetchQuery(query)

    dehydrated = dehydrate(serverQueryClient)

    // --- client ---

    hydrate(clientQueryClient, dehydrated)

    await vi.waitFor(() => promise2)
    await vi.waitFor(() =>
      expect(clientQueryClient.getQueryData(query.queryKey)).toBe(1),
    )

    expect(serializeDataMock).toHaveBeenCalledTimes(2)
    expect(serializeDataMock).toHaveBeenCalledWith(1)

    expect(deserializeDataMock).toHaveBeenCalledTimes(2)
    expect(deserializeDataMock).toHaveBeenCalledWith(1)

    clientQueryClient.clear()
Repository: tannerlinsley/react-query
File: packages/react-query/src/__tests__/mutationOptions.test-d.tsx (TypeScript)

Code before:
import { describe, expectTypeOf, it } from 'vitest'
import { mutationOptions } from '../mutationOptions'

describe('mutationOptions', () => {
  it('should not allow excess properties', () => {
    return mutationOptions({
      mutationFn: () => Promise.resolve(5),
      mutationKey: ['key'],
      // @ts-expect-error this is a good error, because onMutates does not exist!
      onMutates: 1000,
    })
  })

  it('should infer types for callbacks', () => {
    return mutationOptions({
      mutationFn: () => Promise.resolve(5),
      mutationKey: ['key'],
      onSuccess: (data) => {
        expectTypeOf(data).toEqualTypeOf<number>()
      },
    })
  })
})


Review comment (suggestion):
In my opinion, Adding type testing about DataTag would be better. When adding DataTag for queryOptions there is lots of pull requests to fix its typescript error. so if we can guarantee mutationOption's type before releasing this new feature, mutationOptions, it would be better

Code after:
import { assertType, describe, expectTypeOf, it } from 'vitest'
import { QueryClient } from '@tanstack/query-core'
import { useIsMutating, useMutation, useMutationState } from '..'
import { mutationOptions } from '../mutationOptions'
import type {
  DefaultError,
  MutationState,
  WithRequired,
} from '@tanstack/query-core'
import type { UseMutationOptions, UseMutationResult } from '../types'

describe('mutationOptions', () => {
  it('should not allow excess properties', () => {
    // @ts-expect-error this is a good error, because onMutates does not exist!
    mutationOptions({
      mutationFn: () => Promise.resolve(5),
      mutationKey: ['key'],
      onMutates: 1000,
      onSuccess: (data) => {
        expectTypeOf(data).toEqualTypeOf<number>()
      },
    })
  })

  it('should infer types for callbacks', () => {
    mutationOptions({
      mutationFn: () => Promise.resolve(5),
      mutationKey: ['key'],
      onSuccess: (data) => {
        expectTypeOf(data).toEqualTypeOf<number>()
      },
    })
  })

  it('should infer types for onError callback', () => {
    mutationOptions({
      mutationFn: () => {
        throw new Error('fail')
      },
      mutationKey: ['key'],
      onError: (error) => {
        expectTypeOf(error).toEqualTypeOf<DefaultError>()
      },
    })
  })

  it('should infer types for variables', () => {
    mutationOptions<number, DefaultError, { id: string }>({