ronantakizawa/codereview-bench huggingface dataset:
Repository: tannerlinsley/react-query
File: packages/query-core/src/__tests__/streamedQuery.test.tsx (TypeScript)

Code before:

    await vi.advanceTimersByTimeAsync(60)

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

    unsubscribe()

    await vi.advanceTimersByTimeAsync(10)

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

  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,
        }),
        initialValue: {},
      }),
    })

    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,

Review comment (suggestion):
let’s not pass types this way - as it won’t test if type inference works. Instead, let’s type the initialValue:

```
initialValue: {} as Record<number, boolean>
```

Code after:

    await vi.advanceTimersByTimeAsync(60)

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

    unsubscribe()

    await vi.advanceTimersByTimeAsync(10)

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

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

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

    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,
Repository: tannerlinsley/react-query
File: packages/query-core/src/__tests__/streamedQuery.test.tsx (TypeScript)

Code before:

    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()
  })

  test('should support custom reducer with initialValue', 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,
        }),
        initialValue: {
          10: true,
          11: 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',

Review comment (suggestion):
same here please:

```suggestion
      queryFn: streamedQuery({
```

Code after:

    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()
  })

  test('should support custom reducer with initialValue', async () => {
    const key = queryKey()
    const observer = new QueryObserver(queryClient, {
      queryKey: key,
      queryFn: streamedQuery({
        queryFn: () => createAsyncNumberGenerator(2),
        reducer: (acc, chunk) => ({
          ...acc,
          [chunk]: true,
        }),
        initialValue: {
          10: true,
          11: true,
        } as Record<number, boolean>,
      }),
    })

    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',
Repository: tannerlinsley/react-query
File: packages/query-core/src/streamedQuery.ts (TypeScript)

Code before:
    const query = context.client
      .getQueryCache()
      .find({ queryKey: context.queryKey, exact: true })
    const isRefetch = !!query && query.state.data !== undefined
    if (isRefetch && refetchMode === 'reset') {
      query.setState({
        status: 'pending',
        data: undefined,
        error: null,
        fetchStatus: 'fetching',
      })
    }

    let result = initialValue

    const stream = await queryFn(context)

    for await (const chunk of stream) {
      if (context.signal.aborted) {
        break
      }

      // don't append to the cache directly when replace-refetching
      if (!isRefetch || refetchMode !== 'replace') {
        context.client.setQueryData<TData>(context.queryKey, (prev) =>
          reducer(prev ?? initialValue, chunk),
        )
      }
      result = reducer(result, chunk)
    }

    // finalize result: replace-refetching needs to write to the cache
    if (isRefetch && refetchMode === 'replace' && !context.signal.aborted) {
      context.client.setQueryData<TData>(context.queryKey, result)
    }

    return context.client.getQueryData(context.queryKey)!
  }
}


Review comment (suggestion):
we need to explicitly check for `undefined` here because it’s the only indicator for “empty” we have. `null` is technically allowed:

```suggestion
          reducer(prev === undefined ? initialValue : prev, chunk),
```

Code after:
    const query = context.client
      .getQueryCache()
      .find({ queryKey: context.queryKey, exact: true })
    const isRefetch = !!query && query.state.data !== undefined
    if (isRefetch && refetchMode === 'reset') {
      query.setState({
        status: 'pending',
        data: undefined,
        error: null,
        fetchStatus: 'fetching',
      })
    }

    let result = initialValue

    const stream = await queryFn(context)

    for await (const chunk of stream) {
      if (context.signal.aborted) {
        break
      }

      // don't append to the cache directly when replace-refetching
      if (!isRefetch || refetchMode !== 'replace') {
        context.client.setQueryData<TData>(context.queryKey, (prev) =>
          reducer(prev === undefined ? initialValue : prev, chunk),
        )
      }
      result = reducer(result, chunk)
    }

    // finalize result: replace-refetching needs to write to the cache
    if (isRefetch && refetchMode === 'replace' && !context.signal.aborted) {
      context.client.setQueryData<TData>(context.queryKey, result)
    }

    return context.client.getQueryData(context.queryKey)!
  }
}
Repository: tannerlinsley/react-query
File: packages/query-core/src/utils.ts (TypeScript)

Code before:
        aItem === null ||
        bItem === null ||
        typeof aItem !== 'object' ||
        typeof bItem !== 'object'
      ) {
        copy[i] = bItem
        continue
      }

      const v = replaceEqualDeep(aItem, bItem)
      copy[i] = v
      if (v === aItem) equalItems++
    }

    return aSize === bSize && equalItems === aSize ? a : copy
  }

  // only 1 is an array
  if (aIsArr || bIsArr) {
    return b
  }

  // at least 1 is not an object
  if (!isPlainObject(a) || !isPlainObject(b)) {
    return b
  }

  const aSize = Object.keys(a).length
  const copy: Record<PropertyKey, unknown> = {}
  let equalItems = 0
  let bSize = 0

  for (const k in b) {
    bSize++

    const aItem = a[k]
    const bItem = b[k]

    // most common case (strict equality)
    if (aItem === bItem) {
      copy[k] = aItem
      if (hasOwn.call(a, k)) equalItems++
      continue
    }

    // either item is not an array or object
    if (
      aItem === null ||
      bItem === null ||
      typeof aItem !== 'object' ||
      typeof bItem !== 'object'

Review comment (suggestion):
could the “only 1 is an array” case not be removed because we would also go into the “at least 1 is not an object” case and also `return b` there?

all tests still pass if I just remove that check, and we could then also inline `aIsArr` and `bIsArr` because they are only used once then ...

Code after:
    if (
      aItem === null ||
      bItem === null ||
      typeof aItem !== 'object' ||
      typeof bItem !== 'object'
    ) {
      copy[key] = bItem
      continue
    }

    const v = replaceEqualDeep(aItem, bItem)
    copy[key] = v
    if (v === aItem) equalItems++
  }

  return aSize === bSize && equalItems === aSize ? a : copy
}

/**
 * Shallow compare objects.
 */
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
  }

  for (const key in a) {
    if (a[key] !== b[key]) {
      return false
    }
  }

  return true
}

export function isPlainArray(value: unknown): value is Array<unknown> {
  return Array.isArray(value) && value.length === Object.keys(value).length
}

// Copied from: https://github.com/jonschlinkert/is-plain-object
export function isPlainObject(o: any): o is Record<PropertyKey, unknown> {
  if (!hasObjectPrototype(o)) {
    return false
  }

  // If has no constructor
  const ctor = o.constructor
  if (ctor === undefined) {
Repository: tannerlinsley/react-query
File: packages/query-core/src/types.ts (TypeScript)

Code before:
  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 type DropLast<T extends ReadonlyArray<unknown>> = T extends readonly [
  ...infer R,
  unknown,
]
  ? R
  : never

export type TuplePrefixes<T extends ReadonlyArray<unknown>> =
  T extends readonly [] ? readonly [] : TuplePrefixes<DropLast<T>> | T

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

export type DefaultError = Register extends {
  defaultError: infer TError
}
  ? TError
  : Error

export type QueryKey = Register extends {
  queryKey: infer TQueryKey
}
  ? TQueryKey extends ReadonlyArray<unknown>
    ? TQueryKey
    : TQueryKey extends Array<unknown>
      ? TQueryKey
      : ReadonlyArray<unknown>
  : ReadonlyArray<unknown>

Review comment (suggestion):
all those types become part of the public interface, I don’t think we want that so please just inline them in `utils.ts`

Code after:
  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>
  // queryKey: ReadonlyArray<unknown>
  // mutationKey: ReadonlyArray<unknown>
}

export type DefaultError = Register extends {
  defaultError: infer TError
}
  ? TError
  : Error

export type QueryKey = Register extends {
  queryKey: infer TQueryKey
}
  ? TQueryKey extends ReadonlyArray<unknown>
    ? TQueryKey
    : TQueryKey extends Array<unknown>
      ? TQueryKey
      : ReadonlyArray<unknown>
  : ReadonlyArray<unknown>

export const dataTagSymbol = Symbol('dataTagSymbol')
export type dataTagSymbol = typeof dataTagSymbol
export const dataTagErrorSymbol = Symbol('dataTagErrorSymbol')
export type dataTagErrorSymbol = typeof dataTagErrorSymbol
export const unsetMarker = Symbol('unsetMarker')
export type UnsetMarker = typeof unsetMarker
export type AnyDataTag = {
  [dataTagSymbol]: any
  [dataTagErrorSymbol]: any
Repository: tannerlinsley/react-query
File: packages/react-query-devtools/eslint.config.js (JavaScript)

Code before:
// @ts-check

import pluginReact from '@eslint-react/eslint-plugin'
import * as reactHooks from 'eslint-plugin-react-hooks'
import rootConfig from './root.eslint.config.js'

export default [
  ...rootConfig,
  reactHooks.configs.recommended,
  {
    files: ['**/*.{ts,tsx}'],
    ...pluginReact.configs.recommended,
    rules: {
      '@eslint-react/no-context-provider': 'off', // We need to be React 18 compatible
    },
  },
  {
    rules: {
      'react-hooks/exhaustive-deps': 'error',
      'react-hooks/rules-of-hooks': 'error',
      'react-hooks/react-compiler': 'error',
    },
  },
]


Review comment (suggestion):
drive-by comment as I saw your posts on Bluesky...

if you hadn't already come across it, thought it'd be helpful to point to https://github.com/facebook/react/issues/34745, which explains (some of?) the type issues with integrating the plugin's config (and the expect error overrides you needed).

Issue was closed but the fix hasn't been published yet. You may have a better time with integration once it is 😄

Code after:
// @ts-check

import pluginReact from '@eslint-react/eslint-plugin'
import reactHooks from 'eslint-plugin-react-hooks'
import rootConfig from './root.eslint.config.js'

export default [
  ...rootConfig,
  // @ts-expect-error wtf
  ...reactHooks.configs['recommended-latest'],
  {
    files: ['**/*.{ts,tsx}'],
    ...pluginReact.configs.recommended,
    rules: {
      '@eslint-react/no-context-provider': 'off', // We need to be React 18 compatible
      'react-hooks/exhaustive-deps': 'error',
      'react-hooks/rules-of-hooks': 'error',
      'react-hooks/unsupported-syntax': 'error',
      'react-hooks/incompatible-library': 'error',
    },
  },
]
Repository: tannerlinsley/react-query
File: packages/query-core/src/query.ts (TypeScript)

Code before:
  }
  get meta(): QueryMeta | undefined {
    return this.options.meta
  }

  get promise(): Promise<TData> | undefined {
    return this.#retryer?.promise
  }

  setOptions(
    options?: QueryOptions<TQueryFnData, TError, TData, TQueryKey>,
  ): void {
    this.options = { ...this.#defaultOptions, ...options }

    this.updateGcTime(this.options.gcTime)

    // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
    if (this.state && this.state.data === undefined) {
      const defaultState = getDefaultState(this.options)
      if (defaultState.data !== undefined) {
        this.setState({
          ...successState(defaultState.data, defaultState.dataUpdatedAt),
          fetchMeta: null,
          fetchFailureReason: null,
          fetchFailureCount: 0,
        })
        this.#initialState = defaultState
      }
    }
  }

  protected optionalRemove() {
    if (!this.observers.length && this.state.fetchStatus === 'idle') {
      this.#cache.remove(this)
    }
  }

  setData(
    newData: TData,
    options?: SetDataOptions & { manual: boolean },
  ): TData {
    const data = replaceData(this.state.data, newData, this.options)

    // Set data and mark it as cached
    this.#dispatch({
      data,
      type: 'success',
      dataUpdatedAt: options?.updatedAt,
      manual: options?.manual,
    })


Review comment (bug):
```
this.setState(
  successState(defaultState.data, defaultState.dataUpdatedAt),
)
```

The whole idea was to have one defined `successState` so that these two places don’t diverge. I’ll try to fix `fetchMeta` and `fetchFailureCount` separately.

Code after:
  }
  get meta(): QueryMeta | undefined {
    return this.options.meta
  }

  get promise(): Promise<TData> | undefined {
    return this.#retryer?.promise
  }

  setOptions(
    options?: QueryOptions<TQueryFnData, TError, TData, TQueryKey>,
  ): void {
    this.options = { ...this.#defaultOptions, ...options }

    this.updateGcTime(this.options.gcTime)

    // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
    if (this.state && this.state.data === undefined) {
      const defaultState = getDefaultState(this.options)
      if (defaultState.data !== undefined) {
        this.setState(
          successState(defaultState.data, defaultState.dataUpdatedAt),
        )
        this.#initialState = defaultState
      }
    }
  }

  protected optionalRemove() {
    if (!this.observers.length && this.state.fetchStatus === 'idle') {
      this.#cache.remove(this)
    }
  }

  setData(
    newData: TData,
    options?: SetDataOptions & { manual: boolean },
  ): TData {
    const data = replaceData(this.state.data, newData, this.options)

    // Set data and mark it as cached
    this.#dispatch({
      data,
      type: 'success',
      dataUpdatedAt: options?.updatedAt,
      manual: options?.manual,
    })

    return data
  }
Repository: tannerlinsley/react-query
File: packages/solid-query/src/__tests__/queryOptions.test.tsx (TypeScript)

Code before:
import { describe, expect, it } from 'vitest'
import { queryOptions } from '../queryOptions'

describe('queryOptions', () => {
  it('should return the object received as a parameter without any modification.', () => {
    const object = {
      queryKey: ['key'],
      queryFn: () => Promise.resolve(5),
    } as const

    expect(queryOptions(object)).toStrictEqual(object)
  })
})


Review comment (suggestion):
```suggestion
    expect(queryOptions(object)).toBe(object)
```

Code after:
import { describe, expect, it } from 'vitest'
import { queryOptions } from '../queryOptions'

describe('queryOptions', () => {
  it('should return the object received as a parameter without any modification.', () => {
    const object = {
      queryKey: ['key'],
      queryFn: () => Promise.resolve(5),
    } as const

    expect(queryOptions(object)).toBe(object)
  })
})
Repository: tannerlinsley/react-query
File: packages/svelte-query/tests/infiniteQueryOptions/infiniteQueryOptions.svelte.test.ts (TypeScript)

Code before:
import { describe, expect, it } from 'vitest'
import { infiniteQueryOptions } from '../../src/index.js'

describe('infiniteQueryOptions', () => {
  it('should return the object received as a parameter without any modification.', () => {
    const object = {
      queryKey: ['key'],
      queryFn: () => Promise.resolve(5),
      getNextPageParam: () => null,
      initialPageParam: null,
    } as const

    expect(infiniteQueryOptions(object)).toStrictEqual(object)
  })
})


Review comment (suggestion):
```suggestion
    expect(infiniteQueryOptions(object)).toBe(object)
```

Code after:
import { describe, expect, it } from 'vitest'
import { infiniteQueryOptions } from '../../src/index.js'
import type { CreateInfiniteQueryOptions } from '../../src/types.js'

describe('infiniteQueryOptions', () => {
  it('should return the object received as a parameter without any modification.', () => {
    const object: CreateInfiniteQueryOptions = {
      queryKey: ['key'],
      queryFn: () => Promise.resolve(5),
      getNextPageParam: () => null,
      initialPageParam: null,
    } as const

    expect(infiniteQueryOptions(object)).toBe(object)
  })
})
Repository: tannerlinsley/react-query
File: packages/svelte-query/tests/queryOptions/queryOptions.svelte.test.ts (TypeScript)

Code before:
import { describe, expect, it } from 'vitest'
import { queryOptions } from '../../src/index.js'

describe('queryOptions', () => {
  it('should return the object received as a parameter without any modification.', () => {
    const object = {
      queryKey: ['key'],
      queryFn: () => Promise.resolve(5),
    } as const

    expect(queryOptions(object)).toStrictEqual(object)
  })
})


Review comment (suggestion):
```suggestion
    expect(queryOptions(object)).toBe(object)
```

Code after:
import { describe, expect, it } from 'vitest'
import { queryOptions } from '../../src/index.js'

describe('queryOptions', () => {
  it('should return the object received as a parameter without any modification.', () => {
    const object = {
      queryKey: ['key'],
      queryFn: () => Promise.resolve(5),
    } as const

    expect(queryOptions(object)).toBe(object)
  })
})