ronantakizawa/codereview-bench huggingface dataset:
Repository: tannerlinsley/react-query
File: packages/query-core/src/types.ts (TypeScript)

Code before:
/* istanbul ignore file */

import type { QueryClient } from './queryClient'
import type { DehydrateOptions, HydrateOptions } from './hydration'
import type { MutationState } from './mutation'
import type { FetchDirection, Query, QueryBehavior } from './query'
import type { RetryDelayValue, RetryValue } from './retryer'
import type { QueryFilters, QueryTypeFilter, SkipToken } from './utils'
import type { QueryCache } from './queryCache'
import type { MutationCache } from './mutationCache'

export type DistributiveOmit<TKey, TValue> = TKey extends any
  ? Pick<TKey, Exclude<keyof TKey, TValue>>
  : never

export type OmitKeyof<
  TObject,
  TKey extends TStrictly extends 'safely'
    ?
        | keyof TObject
        | (string & Record<never, never>)
        | (number & Record<never, never>)
        | (symbol & Record<never, never>)
    : keyof TObject,
  TStrictly extends 'strictly' | 'safely' = 'strictly',
> = Omit<TObject, TKey>

export type Override<TTargetA, TTargetB> = {
  [AKey in keyof TTargetA]: AKey extends keyof TTargetB
    ? TTargetB[AKey]
    : TTargetA[AKey]
}

export type NoInfer<T> = [T][T extends any ? 0 : never]

export interface Register {
  // defaultError: Error
  // queryMeta: Record<string, unknown>
  // mutationMeta: Record<string, unknown>

Review comment (suggestion):
it's not a big deal, but how about below naming like below?

```suggestion
export type DistributiveOmit<
  TObject,
  TKey extends keyof any,
> = TObject extends any ? Omit<TObject, TKey> : never
```

There was same one in @tanstack/vue-query 

https://github.com/TanStack/query/blob/89846b71eb286cdb05858ad06e1344a9206564ce/packages/vue-query/src/types.ts#L58-L60

Code after:
/* istanbul ignore file */

import type { QueryClient } from './queryClient'
import type { DehydrateOptions, HydrateOptions } from './hydration'
import type { MutationState } from './mutation'
import type { FetchDirection, Query, QueryBehavior } from './query'
import type { RetryDelayValue, RetryValue } from './retryer'
import type { QueryFilters, QueryTypeFilter, SkipToken } from './utils'
import type { QueryCache } from './queryCache'
import type { MutationCache } from './mutationCache'

export type DistributiveOmit<
  TObject,
  TKey extends keyof TObject,
> = TObject extends any ? Omit<TObject, TKey> : never

export type OmitKeyof<
  TObject,
  TKey extends TStrictly extends 'safely'
    ?
        | keyof TObject
        | (string & Record<never, never>)
        | (number & Record<never, never>)
        | (symbol & Record<never, never>)
    : keyof TObject,
  TStrictly extends 'strictly' | 'safely' = 'strictly',
> = Omit<TObject, TKey>

export type Override<TTargetA, TTargetB> = {
  [AKey in keyof TTargetA]: AKey extends keyof TTargetB
    ? TTargetB[AKey]
    : TTargetA[AKey]
}

export type NoInfer<T> = [T][T extends any ? 0 : never]

export interface Register {
  // defaultError: Error
  // queryMeta: Record<string, unknown>
Repository: tannerlinsley/react-query
File: packages/query-core/src/types.ts (TypeScript)

Code before:
/* istanbul ignore file */

import type { QueryClient } from './queryClient'
import type { DehydrateOptions, HydrateOptions } from './hydration'
import type { MutationState } from './mutation'
import type { FetchDirection, Query, QueryBehavior } from './query'
import type { RetryDelayValue, RetryValue } from './retryer'
import type { QueryFilters, QueryTypeFilter, SkipToken } from './utils'
import type { QueryCache } from './queryCache'
import type { MutationCache } from './mutationCache'

export type DistributiveOmit<TObject, TKey> = TObject extends any
  ? Pick<TObject, Exclude<keyof TObject, TKey>>
  : never

export type OmitKeyof<
  TObject,
  TKey extends TStrictly extends 'safely'
    ?
        | keyof TObject
        | (string & Record<never, never>)
        | (number & Record<never, never>)
        | (symbol & Record<never, never>)
    : keyof TObject,
  TStrictly extends 'strictly' | 'safely' = 'strictly',
> = Omit<TObject, TKey>

export type Override<TTargetA, TTargetB> = {
  [AKey in keyof TTargetA]: AKey extends keyof TTargetB
    ? TTargetB[AKey]
    : TTargetA[AKey]
}

export type NoInfer<T> = [T][T extends any ? 0 : never]

export interface Register {
  // defaultError: Error
  // queryMeta: Record<string, unknown>
  // mutationMeta: Record<string, unknown>

Review comment (suggestion):
```suggestion
export type DistributiveOmit<
  TObject,
  TKey extends keyof TObject,
> = TObject extends any ? Omit<TObject, TKey> : never
```

By doing this, we can change it when we hover to the following:

### AS-IS (verbose)

<img width="621" alt="image" src="https://github.com/user-attachments/assets/276cdd2e-38f7-489a-aaeb-9bfd116fde42" />

### TO-BE (simple)

<img width="627" alt="image" src="https://github.com/user-attachments/assets/c62db321-7c00-4c8b-a076-d13c073cddb9" />



Code after:
/* istanbul ignore file */

import type { QueryClient } from './queryClient'
import type { DehydrateOptions, HydrateOptions } from './hydration'
import type { MutationState } from './mutation'
import type { FetchDirection, Query, QueryBehavior } from './query'
import type { RetryDelayValue, RetryValue } from './retryer'
import type { QueryFilters, QueryTypeFilter, SkipToken } from './utils'
import type { QueryCache } from './queryCache'
import type { MutationCache } from './mutationCache'

export type DistributiveOmit<
  TObject,
  TKey extends keyof TObject,
> = TObject extends any ? Omit<TObject, TKey> : never

export type OmitKeyof<
  TObject,
  TKey extends TStrictly extends 'safely'
    ?
        | keyof TObject
        | (string & Record<never, never>)
        | (number & Record<never, never>)
        | (symbol & Record<never, never>)
    : keyof TObject,
  TStrictly extends 'strictly' | 'safely' = 'strictly',
> = Omit<TObject, TKey>

export type Override<TTargetA, TTargetB> = {
  [AKey in keyof TTargetA]: AKey extends keyof TTargetB
    ? TTargetB[AKey]
    : TTargetA[AKey]
}

export type NoInfer<T> = [T][T extends any ? 0 : never]

export interface Register {
  // defaultError: Error
  // queryMeta: Record<string, unknown>
Repository: tannerlinsley/react-query
File: packages/query-core/src/__tests__/query.test.tsx (TypeScript)

Code before:
      structuralSharing: () => {
        throw Error('Any error')
      },
    })

    const query = queryCache.find({ queryKey: key })!

    expect(queryFn).toHaveBeenCalledTimes(1)
    await vi.advanceTimersByTimeAsync(10)
    expect(query.state.status).toBe('error')
  })

  test('should use persister if provided', async () => {
    const key = queryKey()

    await queryClient.prefetchQuery({
      queryKey: key,
      queryFn: () => 'data',
      persister: () => Promise.resolve('persisted data'),
    })

    const query = queryCache.find({ queryKey: key })!
    expect(query.state.data).toBe('persisted data')
  })

  test('should use queryFn from observer if not provided in options', async () => {
    const key = queryKey()
    const observer = new QueryObserver(queryClient, {
      queryKey: key,
      queryFn: () => Promise.resolve('data'),
    })

    const query = new Query({
      client: queryClient,
      queryKey: key,
      queryHash: hashQueryKeyByOptions(key),
    })

    query.addObserver(observer)

    await query.fetch()
    const result = await query.state.data
    expect(result).toBe('data')
  })

  test('should log error when queryKey is not an array', async () => {
    const consoleMock = vi.spyOn(console, 'error')
    const key: unknown = 'string-key'

    await queryClient.prefetchQuery({
      queryKey: key as QueryKey,

Review comment (suggestion):
we could also assert here that `query.options.queryFn` is set to the one of the observer at the end.

Code after:
      structuralSharing: () => {
        throw Error('Any error')
      },
    })

    const query = queryCache.find({ queryKey: key })!

    expect(queryFn).toHaveBeenCalledTimes(1)
    await vi.advanceTimersByTimeAsync(10)
    expect(query.state.status).toBe('error')
  })

  test('should use persister if provided', async () => {
    const key = queryKey()

    await queryClient.prefetchQuery({
      queryKey: key,
      queryFn: () => 'data',
      persister: () => Promise.resolve('persisted data'),
    })

    const query = queryCache.find({ queryKey: key })!
    expect(query.state.data).toBe('persisted data')
  })

  test('should use queryFn from observer if not provided in options', async () => {
    const key = queryKey()
    const queryFn = () => Promise.resolve('data')
    const observer = new QueryObserver(queryClient, {
      queryKey: key,
      queryFn: queryFn,
    })

    const query = new Query({
      client: queryClient,
      queryKey: key,
      queryHash: hashQueryKeyByOptions(key),
    })

    query.addObserver(observer)

    await query.fetch()
    const result = await query.state.data
    expect(result).toBe('data')
    expect(query.options.queryFn).toBe(queryFn)
  })

  test('should log error when queryKey is not an array', async () => {
    const consoleMock = vi.spyOn(console, 'error')
    const key: unknown = 'string-key'
Repository: tannerlinsley/react-query
File: packages/query-core/src/thenable.ts (TypeScript)

Code before:

    reject(reason)
  }

  return thenable
}

/**
 * This function takes a Promise-like input and detects whether the data
 * is synchronously available or not.
 *
 * It does not inspect .status, .value or .reason properties of the promise,
 * as those are not always available, and the .status of React's promises
 * should not be considered part of the public API.
 */
export function tryResolveSync(promise: Promise<unknown> | Thenable<unknown>) {
  let data: unknown

  promise
    .then((result) => {
      data = result
      return result
    })
    // This can be unavailable on certain kinds of thenable's
    // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
    ?.catch(() => {})

  if (data !== undefined) {
    return { data }
  }

  return undefined
}


Review comment (suggestion):
using `noop` from `query-core` should get rid of the eslint-disable:

```suggestion
    ?.catch(noop)
```

Code after:
      reason,
    })

    reject(reason)
  }

  return thenable
}

/**
 * This function takes a Promise-like input and detects whether the data
 * is synchronously available or not.
 *
 * It does not inspect .status, .value or .reason properties of the promise,
 * as those are not always available, and the .status of React's promises
 * should not be considered part of the public API.
 */
export function tryResolveSync(promise: Promise<unknown> | Thenable<unknown>) {
  let data: unknown

  promise
    .then((result) => {
      data = result
      return result
    })
    // .catch can be unavailable on certain kinds of thenable's
    // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
    ?.catch(noop)

  if (data !== undefined) {
    return { data }
  }

  return undefined
}
Repository: tannerlinsley/react-query
File: packages/query-core/src/types.ts (TypeScript)

Code before:
  ) => void
}

export type MutateFunction<
  TData = unknown,
  TError = unknown,
  TVariables = void,
  TContext = unknown,
> = (
  variables: TVariables,
  options?: MutateOptions<TData, TError, TVariables, TContext>,
) => Promise<TData>

export interface MutationObserverBaseResult<
  TData = unknown,
  TError = unknown,
  TVariables = void,
  TContext = unknown,
> extends MutationState<TData, TError, TVariables, TContext> {
  isError: boolean
  isIdle: boolean
  isLoading: boolean
  isPending: boolean
  isSuccess: boolean
  mutate: MutateFunction<TData, TError, TVariables, TContext>
  reset: () => void
}

export interface MutationObserverIdleResult<
  TData = unknown,
  TError = unknown,
  TVariables = void,
  TContext = unknown,
> extends MutationObserverBaseResult<TData, TError, TVariables, TContext> {
  data: undefined
  error: null
  isError: false
  isIdle: true
  isLoading: false
  isSuccess: false
  status: 'idle'
}

export interface MutationObserverLoadingResult<
  TData = unknown,
  TError = unknown,
  TVariables = void,
  TContext = unknown,
> extends MutationObserverBaseResult<TData, TError, TVariables, TContext> {
  data: undefined
  error: null

Review comment (suggestion):
We can now see a strikethrough line on `isLoading`.

<img width="571" alt="image" src="https://github.com/user-attachments/assets/f2b4dce0-e92e-4cd4-85c9-4711ead75d79" />


Code after:
  ) => void
}

export type MutateFunction<
  TData = unknown,
  TError = unknown,
  TVariables = void,
  TContext = unknown,
> = (
  variables: TVariables,
  options?: MutateOptions<TData, TError, TVariables, TContext>,
) => Promise<TData>

export interface MutationObserverBaseResult<
  TData = unknown,
  TError = unknown,
  TVariables = void,
  TContext = unknown,
> extends MutationState<TData, TError, TVariables, TContext> {
  isError: boolean
  isIdle: boolean
  /**
   * @deprecated This property will be removed in the next major version. Use `isPending` instead.
   */
  isLoading: boolean
  isPending: boolean
  isSuccess: boolean
  mutate: MutateFunction<TData, TError, TVariables, TContext>
  reset: () => void
}

export interface MutationObserverIdleResult<
  TData = unknown,
  TError = unknown,
  TVariables = void,
  TContext = unknown,
> extends MutationObserverBaseResult<TData, TError, TVariables, TContext> {
  data: undefined
  error: null
  isError: false
  isIdle: true
  /**
   * @deprecated This property will be removed in the next major version. Use `isPending` instead.
   */
  isLoading: false
  isPending: false
  isSuccess: false
  status: 'idle'
}

export interface MutationObserverLoadingResult<
Repository: tannerlinsley/react-query
File: packages/react-query/src/errorBoundaryUtils.ts (TypeScript)

Code before:
import { shouldThrowError } from '@tanstack/query-core'
import type {
  DefaultedQueryObserverOptions,
  Query,
  QueryKey,
  QueryObserverResult,
  ThrowOnError,
} from '@tanstack/query-core'
import type { QueryErrorResetBoundaryValue } from './QueryErrorResetBoundary'

export const ensurePreventErrorBoundaryRetry = <
  TQueryFnData,
  TError,
  TData,
  TQueryData,
  TQueryKey extends QueryKey,
>(
  options: DefaultedQueryObserverOptions<
    TQueryFnData,
    TError,
    TData,
    TQueryData,
    TQueryKey
  >,
  errorResetBoundary: QueryErrorResetBoundaryValue,
  query: Query<TQueryFnData, TError, TQueryData, TQueryKey>,
) => {
  const throwOnError =
    query.state.error && typeof options.throwOnError === 'function'
      ? shouldThrowError(options.throwOnError, [query.state.error, query])
      : options.throwOnError

  if (
    options.suspense ||
    options.experimental_prefetchInRender ||
    throwOnError
  ) {
    // Prevent retrying failed query if the error boundary has not been reset yet
    if (!errorResetBoundary.isReset()) {
      options.retryOnMount = false
    }
  }
}

export const useClearResetErrorBoundary = (
  errorResetBoundary: QueryErrorResetBoundaryValue,
) => {
  React.useEffect(() => {
    errorResetBoundary.clearReset()
  }, [errorResetBoundary])
}

Review comment (suggestion):
just seeing this now: to be on par with `getHasError`, we should make this param required, but potentially undefined, and then do a single check in here. sorry about changing my mind here 😅 


```suggestion
  query: Query<TQueryFnData, TError, TQueryData, TQueryKey> | undefined,
```

Code after:
import { shouldThrowError } from '@tanstack/query-core'
import type {
  DefaultedQueryObserverOptions,
  Query,
  QueryKey,
  QueryObserverResult,
  ThrowOnError,
} from '@tanstack/query-core'
import type { QueryErrorResetBoundaryValue } from './QueryErrorResetBoundary'

export const ensurePreventErrorBoundaryRetry = <
  TQueryFnData,
  TError,
  TData,
  TQueryData,
  TQueryKey extends QueryKey,
>(
  options: DefaultedQueryObserverOptions<
    TQueryFnData,
    TError,
    TData,
    TQueryData,
    TQueryKey
  >,
  errorResetBoundary: QueryErrorResetBoundaryValue,
  query: Query<TQueryFnData, TError, TQueryData, TQueryKey> | undefined,
) => {
  const throwOnError =
    query?.state.error && typeof options.throwOnError === 'function'
      ? shouldThrowError(options.throwOnError, [query.state.error, query])
      : options.throwOnError

  if (
    options.suspense ||
    options.experimental_prefetchInRender ||
    throwOnError
  ) {
    // Prevent retrying failed query if the error boundary has not been reset yet
    if (!errorResetBoundary.isReset()) {
      options.retryOnMount = false
    }
  }
}

export const useClearResetErrorBoundary = (
  errorResetBoundary: QueryErrorResetBoundaryValue,
) => {
  React.useEffect(() => {
    errorResetBoundary.clearReset()
  }, [errorResetBoundary])
}
Repository: tannerlinsley/react-query
File: packages/angular-query-experimental/src/__tests__/inject-is-mutating.test.ts (TypeScript)

Code before:
  })

  afterEach(() => {
    vi.useRealTimers()
  })

  test('should properly return isMutating state', async () => {
    const [mutation, isMutating] = TestBed.runInInjectionContext(() => [
      injectMutation(() => ({
        mutationKey: ['isMutating1'],
        mutationFn: (params: { par1: string }) => sleep(10).then(() => params),
      })),
      injectIsMutating(),
    ])

    mutation.mutate({
      par1: 'par1',
    })

    expect(isMutating()).toBe(0)
    await vi.advanceTimersByTimeAsync(0)
    expect(isMutating()).toBe(1)
    await vi.advanceTimersByTimeAsync(11)
    expect(isMutating()).toBe(0)
  })

  describe('injection context', () => {
    test('throws NG0203 with descriptive error outside injection context', () => {
      expect(() => {
        injectIsMutating()
      }).toThrowError(/NG0203(.*?)injectIsMutating/)
    })

    test('can be used outside injection context when passing an injector', () => {
      expect(
        injectIsMutating(undefined, {
          injector: TestBed.inject(Injector),
        }),
      ).not.toThrow()
    })
  })
})


Review comment (suggestion):
```suggestion
    expect(isMutating()).toBe(0)
    mutation.mutate({
      par1: 'par1',
    })

    expect(isMutating()).toBe(0)
    await vi.advanceTimersByTimeAsync(0)
    expect(isMutating()).toBe(1)
    await vi.advanceTimersByTimeAsync(11)
    expect(isMutating()).toBe(0)
```

Code after:
  })

  afterEach(() => {
    vi.useRealTimers()
  })

  test('should properly return isMutating state', async () => {
    const [mutation, isMutating] = TestBed.runInInjectionContext(() => [
      injectMutation(() => ({
        mutationKey: ['isMutating1'],
        mutationFn: (params: { par1: string }) => sleep(10).then(() => params),
      })),
      injectIsMutating(),
    ])

    expect(isMutating()).toBe(0)

    mutation.mutate({
      par1: 'par1',
    })

    expect(isMutating()).toBe(0)
    await vi.advanceTimersByTimeAsync(0)
    expect(isMutating()).toBe(1)
    await vi.advanceTimersByTimeAsync(11)
    expect(isMutating()).toBe(0)
  })

  describe('injection context', () => {
    test('throws NG0203 with descriptive error outside injection context', () => {
      expect(() => {
        injectIsMutating()
      }).toThrowError(/NG0203(.*?)injectIsMutating/)
    })

    test('can be used outside injection context when passing an injector', () => {
      expect(
        injectIsMutating(undefined, {
          injector: TestBed.inject(Injector),
        }),
      ).not.toThrow()
    })
  })
})
Repository: tannerlinsley/react-query
File: packages/svelte-query/vite.config.ts (TypeScript)

Code before:
import { svelteTesting } from '@testing-library/svelte/vite'

import packageJson from './package.json'

export default defineConfig({
  plugins: [svelte(), svelteTesting()],
  // fix from https://github.com/vitest-dev/vitest/issues/6992#issuecomment-2509408660
  resolve: {
    conditions: ['@tanstack/custom-condition'],
  },
  environments: {
    ssr: {
      resolve: {
        conditions: ['@tanstack/custom-condition'],
      },
    },
  },
  test: {
    name: packageJson.name,
    dir: './tests',
    watch: false,
    environment: 'jsdom',
    setupFiles: ['./tests/test-setup.ts'],
    coverage: { enabled: false, provider: 'istanbul', include: ['src/**/*'] },
    typecheck: { enabled: true },
    globals: true,
  },
})


Review comment (refactor):
Can you please remove this? I've confirmed vitest still runs without it. Also, there are a few eslint errors (one was actually in the target branch which I've just fixed).

Code after:
import { svelteTesting } from '@testing-library/svelte/vite'

import packageJson from './package.json'

export default defineConfig({
  plugins: [svelte(), svelteTesting()],
  // fix from https://github.com/vitest-dev/vitest/issues/6992#issuecomment-2509408660
  resolve: {
    conditions: ['@tanstack/custom-condition'],
  },
  environments: {
    ssr: {
      resolve: {
        conditions: ['@tanstack/custom-condition'],
      },
    },
  },
  test: {
    name: packageJson.name,
    dir: './tests',
    watch: false,
    environment: 'jsdom',
    setupFiles: ['./tests/test-setup.ts'],
    coverage: { enabled: false, provider: 'istanbul', include: ['src/**/*'] },
    typecheck: { enabled: true },
  },
})
Repository: tannerlinsley/react-query
File: packages/query-core/src/__tests__/streamedQuery.test.tsx (TypeScript)

Code before:
    expect(observer.getCurrentResult()).toMatchObject({
      status: 'success',
      fetchStatus: 'fetching',
      data: [0, 1, 0],
    })

    await vi.advanceTimersByTimeAsync(100)

    expect(observer.getCurrentResult()).toMatchObject({
      status: 'success',
      fetchStatus: 'idle',
      data: [0, 1, 0, 1, 2],
    })

    unsubscribe()
  })

  test('should support custom reducer', async () => {
    const key = queryKey()

    const observer = new QueryObserver(queryClient, {
      queryKey: key,
      queryFn: streamedQuery<number, Record<number, boolean>>({
        queryFn: () => createAsyncNumberGenerator(2),
        reducer: (acc, chunk) => ({
            ...acc,
            [chunk]: true
          }),
      })
    })

    const unsubscribe = observer.subscribe(vi.fn())

    expect(observer.getCurrentResult()).toMatchObject({
      status: 'pending',
      fetchStatus: 'fetching',
      data: undefined,
    })

    await vi.advanceTimersByTimeAsync(100)

    expect(observer.getCurrentResult()).toMatchObject({
      status: 'success',
      fetchStatus: 'idle',
      data: {
        0: true, 
        1: true
      },
    })

    unsubscribe()

Review comment (suggestion):
the first time `reducer` is invoked, `acc` is an empty array, because `placeholderData` is not specified.
it seems a bit strange to me. but i'm not sure how to solve it. 
i can apply the same logic as in native `reduce`... 
 
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce#initialvalue.


Code after:
    expect(observer.getCurrentResult()).toMatchObject({
      status: 'success',
      fetchStatus: 'fetching',
      data: [0, 1, 0],
    })

    await vi.advanceTimersByTimeAsync(100)

    expect(observer.getCurrentResult()).toMatchObject({
      status: 'success',
      fetchStatus: 'idle',
      data: [0, 1, 0, 1, 2],
    })

    unsubscribe()
  })

  test('should abort when unsubscribed', async () => {
    const key = queryKey()
    const observer = new QueryObserver(queryClient, {
      queryKey: key,
      queryFn: streamedQuery({
        queryFn: (context) => {
          // just consume the signal
          // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
          const numbers = context.signal ? 3 : 0
          return createAsyncNumberGenerator(numbers)
        },
      }),
    })

    const unsubscribe = observer.subscribe(vi.fn())

    expect(queryClient.getQueryState(key)).toMatchObject({
      status: 'pending',
      fetchStatus: 'fetching',
      data: undefined,
    })

    await vi.advanceTimersByTimeAsync(60)

    expect(queryClient.getQueryState(key)).toMatchObject({
      status: 'success',
      fetchStatus: 'fetching',
      data: [0],
    })

    unsubscribe()

    await vi.advanceTimersByTimeAsync(10)