ronantakizawa/codereview-bench huggingface dataset:
Repository: tanstack/query
File: packages/query-core/src/hydration.ts (TypeScript)

Code before:
    queryHash: query.queryHash,
    ...(query.state.status === 'pending' && {
      promise: query.promise?.catch((error) => {
        if (process.env.NODE_ENV !== 'production') {
          console.error(
            `A query that was dehydrated as pending ended up rejecting. [${query.queryHash}]: ${error}; The error will be redacted in production builds`,
          )
        }
        return Promise.reject(new Error('redacted'))
      }),
    }),
    ...(query.meta && { meta: query.meta }),
  }
}

export function defaultShouldDehydrateMutation(mutation: Mutation) {
  return mutation.state.isPaused
}

export function defaultShouldDehydrateQuery(query: Query) {
  return query.state.status === 'success'
}

export function defaultDeserialize(data: unknown) {
  return data
}

export function dehydrate(
  client: QueryClient,
  options: DehydrateOptions = {},
): DehydratedState {
  const filterMutation =
    options.shouldDehydrateMutation ??
    client.getDefaultOptions().dehydrate?.shouldDehydrateMutation ??
    defaultShouldDehydrateMutation

  const mutations = client
    .getMutationCache()
    .getAll()
    .flatMap((mutation) =>
      filterMutation(mutation) ? [dehydrateMutation(mutation)] : [],
    )

  const filterQuery =
    options.shouldDehydrateQuery ??
    client.getDefaultOptions().dehydrate?.shouldDehydrateQuery ??
    defaultShouldDehydrateQuery

  const queries = client
    .getQueryCache()
    .getAll()

Review comment (suggestion):
didn't find any existing `identity` functions when i did a quick search. lmk if you have one somewhere already

Code after:
    queryHash: query.queryHash,
    ...(query.state.status === 'pending' && {
      promise: query.promise?.catch((error) => {
        if (process.env.NODE_ENV !== 'production') {
          console.error(
            `A query that was dehydrated as pending ended up rejecting. [${query.queryHash}]: ${error}; The error will be redacted in production builds`,
          )
        }
        return Promise.reject(new Error('redacted'))
      }),
    }),
    ...(query.meta && { meta: query.meta }),
  }
}

export function defaultShouldDehydrateMutation(mutation: Mutation) {
  return mutation.state.isPaused
}

export function defaultShouldDehydrateQuery(query: Query) {
  return query.state.status === 'success'
}

export function dehydrate(
  client: QueryClient,
  options: DehydrateOptions = {},
): DehydratedState {
  const filterMutation =
    options.shouldDehydrateMutation ??
    client.getDefaultOptions().dehydrate?.shouldDehydrateMutation ??
    defaultShouldDehydrateMutation

  const mutations = client
    .getMutationCache()
    .getAll()
    .flatMap((mutation) =>
      filterMutation(mutation) ? [dehydrateMutation(mutation)] : [],
    )

  const filterQuery =
    options.shouldDehydrateQuery ??
    client.getDefaultOptions().dehydrate?.shouldDehydrateQuery ??
    defaultShouldDehydrateQuery

  const queries = client
    .getQueryCache()
    .getAll()
    .flatMap((query) => (filterQuery(query) ? [dehydrateQuery(query)] : []))

  return { mutations, queries }
}
Repository: tanstack/query
File: packages/query-core/src/hydration.ts (TypeScript)

Code before:
        client,
        {
          ...client.getDefaultOptions().hydrate?.queries,
          ...options?.defaultOptions?.queries,
          queryKey,
          queryHash,
          meta,
        },
        // Reset fetch status to idle to avoid
        // query being stuck in fetching state upon hydration
        {
          ...state,
          fetchStatus: 'idle',
        },
      )
    }

    if (promise) {
      const transformPromise =
        client.getDefaultOptions().hydrate?.transformPromise ??
        defaultTransformPromise

      // Note: `Promise.resolve` required cause
      // RSC transformed promises are not thenable
      const initialPromise = transformPromise(Promise.resolve(promise))

      // this doesn't actually fetch - it just creates a retryer
      // which will re-use the passed `initialPromise`
      void query.fetch(undefined, { initialPromise })
    }
  })
}


Review comment (suggestion):
as discussed in Discord, let's change this to:

```
const transformPromise = client.getDefaultOptions.hydrate?.transformPromise

// Note: `Promise.resolve` required cause
// RSC transformed promises are not thenable
const initialPromise = transformPromise?.(Promise.resolve(promise)) ?? promise
```

That also means we don't need `defaultTransformPromise`

Code after:
          queryKey,
          queryHash,
          meta,
        },
        // Reset fetch status to idle to avoid
        // query being stuck in fetching state upon hydration
        {
          ...state,
          fetchStatus: 'idle',
        },
      )
    }

    if (promise) {
      const transformPromise =
        client.getDefaultOptions().hydrate?.transformPromise

      // Note: `Promise.resolve` required cause
      // RSC transformed promises are not thenable
      const initialPromise =
        transformPromise?.(Promise.resolve(promise)) ?? promise

      // this doesn't actually fetch - it just creates a retryer
      // which will re-use the passed `initialPromise`
      void query.fetch(undefined, { initialPromise })
    }
  })
}
Repository: tanstack/query
File: packages/query-core/src/hydration.ts (TypeScript)

Code before:
        client,
        {
          ...client.getDefaultOptions().hydrate?.queries,
          ...options?.defaultOptions?.queries,
          queryKey,
          queryHash,
          meta,
        },
        // Reset fetch status to idle to avoid
        // query being stuck in fetching state upon hydration
        {
          ...state,
          fetchStatus: 'idle',
        },
      )
    }

    if (promise) {
      const { transformPromise } = client.getDefaultOptions().hydrate ?? {}

      // Note: `Promise.resolve` required cause
      // RSC transformed promises are not thenable
      const initialPromise = transformPromise
        ? transformPromise(Promise.resolve(promise))
        : promise

      // this doesn't actually fetch - it just creates a retryer
      // which will re-use the passed `initialPromise`
      void query.fetch(undefined, { initialPromise })
    }
  })
}


Review comment (suggestion):
not a bit fan of using `{}` as a fallback because it might widen the type to `| {}`. We could just keep it undefined and then use optional chaining as well:

```suggestion
      const transformPromise =
        client.getDefaultOptions().hydrate?.transformPromise

      // Note: `Promise.resolve` required cause
      // RSC transformed promises are not thenable
      const initialPromise =
        transformPromise?.(Promise.resolve(promise)) ?? promise

```

Code after:
        client,
        {
          ...client.getDefaultOptions().hydrate?.queries,
          ...options?.defaultOptions?.queries,
          queryKey,
          queryHash,
          meta,
        },
        // Reset fetch status to idle to avoid
        // query being stuck in fetching state upon hydration
        {
          ...state,
          fetchStatus: 'idle',
        },
      )
    }

    if (promise) {
      const transformPromise =
        client.getDefaultOptions().hydrate?.transformPromise

      // Note: `Promise.resolve` required cause
      // RSC transformed promises are not thenable
      const initialPromise =
        transformPromise?.(Promise.resolve(promise)) ?? promise

      // this doesn't actually fetch - it just creates a retryer
      // which will re-use the passed `initialPromise`
      void query.fetch(undefined, { initialPromise })
    }
  })
}
Repository: tanstack/query
File: packages/react-query/src/__tests__/prefetch.test.tsx (TypeScript)

Code before:
      return (
        <div>
          <div>data: {String(state.data)}</div>
          <div>fetching: {state.isFetching ? 'true' : 'false'}</div>
        </div>
      )
    }

    function App() {
      usePrefetchQuery({ ...queryOpts, queryFn: anotherSpy })

      return (
        <React.Suspense fallback="Loading...">
          <Suspended />
        </React.Suspense>
      )
    }

    await queryClient.fetchQuery(queryOpts)
    const rendered = renderWithClient(queryClient, <App />)

    expect(rendered.queryByText('fetching: true')).not.toBeInTheDocument()
    expect(
      rendered.getByText('data: Prefetch is really nice!'),
    ).toBeInTheDocument()
    expect(anotherSpy).not.toHaveBeenCalled()
  })

  it('should not create a suspense waterfall if prefetch is fired', async () => {
    const firstQueryOpts = {
      queryKey: queryKey(),
      queryFn: vi.fn().mockImplementation(async () => {
        await sleep(10)

        return 'Prefetch is nice!'
      }),
    }

    const secondQueryOpts = {
      queryKey: queryKey(),
      queryFn: vi.fn().mockImplementation(async () => {
        await sleep(10)

        return 'Prefetch is really nice!!'
      }),
    }

    const thirdQueryOpts = {
      queryKey: queryKey(),
      queryFn: vi.fn().mockImplementation(async () => {
        await sleep(10)

Review comment (suggestion):
let's also add:

```suggestion
    expect(anotherSpy).not.toHaveBeenCalled()
    expect(queryOpts.queryFn).not.toHaveBeenCalled()
```

Code after:
describe('usePrefetchQuery', () => {
  const queryCache = new QueryCache()
  const queryClient = createQueryClient({ queryCache })

  function Suspended<T = unknown>(props: {
    queryOpts: UseQueryOptions<T, Error, T, Array<string>>
  }) {
    const state = useSuspenseQuery(props.queryOpts)

    return (
      <div>
        <div>data: {String(state.data)}</div>
      </div>
    )
  }

  it('should prefetch query if query state does not exist', async () => {
    const queryOpts = createQueryOptions({
      data: 'Prefetch is nice!',
    })

    function App() {
      usePrefetchQuery(queryOpts)

      return (
        <React.Suspense fallback="Loading...">
          <Suspended queryOpts={queryOpts} />
        </React.Suspense>
      )
    }

    const rendered = renderWithClient(queryClient, <App />)

    await waitFor(() => rendered.getByText('data: Prefetch is nice!'))
    expect(queryOpts.queryFn).toHaveBeenCalledTimes(1)
  })

  it('should not prefetch query if query state exists', async () => {
    const queryOpts = createQueryOptions({ data: 'Prefetch is really nice!' })

    function App() {
      usePrefetchQuery(queryOpts)

      return (
        <React.Suspense fallback="Loading...">
          <Suspended queryOpts={queryOpts} />
        </React.Suspense>
      )
    }

    await queryClient.fetchQuery(queryOpts)
Repository: tanstack/query
File: packages/react-query/src/__tests__/prefetch.test.tsx (TypeScript)

Code before:

      throw new Error('Oops! Server error!')
    })

    function App() {
      usePrefetchQuery(queryOpts)

      return (
        <ErrorBoundary fallbackRender={() => <div>Oops!</div>}>
          <React.Suspense fallback="Loading...">
            <Suspended queryOpts={queryOpts} />
          </React.Suspense>
        </ErrorBoundary>
      )
    }

    await queryClient.prefetchQuery(queryOpts)
    queryFn.mockClear()
    const rendered = renderWithClient(queryClient, <App />)

    await waitFor(() => rendered.getByText('Oops!'))
    expect(rendered.queryByText('data: Not an error')).not.toBeInTheDocument()
    expect(queryOpts.queryFn).not.toHaveBeenCalled()
  })

  it('should not create a endless loop when using inside a suspense boundary', async () => {
    const queryFn = generateQueryFn('prefetchedQuery')

    const queryOpts = {
      queryKey: queryKey(),
      queryFn,
    }

    function Prefetch({ children }: { children: React.ReactNode }) {
      usePrefetchQuery(queryOpts)
      return <>{children}</>
    }

    function App() {
      return (
        <React.Suspense>
          <Prefetch>
            <Suspended queryOpts={queryOpts} />
          </Prefetch>
        </React.Suspense>
      )
    }

    const rendered = renderWithClient(queryClient, <App />)
    await waitFor(() => rendered.getByText('data: prefetchedQuery'))
    expect(queryOpts.queryFn).toHaveBeenCalledTimes(1)

Review comment (suggestion):
```suggestion
  it('should not create an endless loop when using inside a suspense boundary', async () => {
```

Code after:

      throw new Error('Oops! Server error!')
    })

    function App() {
      usePrefetchQuery(queryOpts)

      return (
        <ErrorBoundary fallbackRender={() => <div>Oops!</div>}>
          <React.Suspense fallback="Loading...">
            <Suspended queryOpts={queryOpts} />
          </React.Suspense>
        </ErrorBoundary>
      )
    }

    await queryClient.prefetchQuery(queryOpts)
    queryFn.mockClear()
    const rendered = renderWithClient(queryClient, <App />)

    await waitFor(() => rendered.getByText('Oops!'))
    expect(rendered.queryByText('data: Not an error')).not.toBeInTheDocument()
    expect(queryOpts.queryFn).not.toHaveBeenCalled()
  })

  it('should not create an endless loop when using inside a suspense boundary', async () => {
    const queryFn = generateQueryFn('prefetchedQuery')

    const queryOpts = {
      queryKey: queryKey(),
      queryFn,
    }

    function Prefetch({ children }: { children: React.ReactNode }) {
      usePrefetchQuery(queryOpts)
      return <>{children}</>
    }

    function App() {
      return (
        <React.Suspense>
          <Prefetch>
            <Suspended queryOpts={queryOpts} />
          </Prefetch>
        </React.Suspense>
      )
    }

    const rendered = renderWithClient(queryClient, <App />)
    await waitFor(() => rendered.getByText('data: prefetchedQuery'))
    expect(queryOpts.queryFn).toHaveBeenCalledTimes(1)
Repository: tanstack/query
File: packages/query-core/src/infiniteQueryObserver.ts (TypeScript)

Code before:
import type { QueryClient } from './queryClient'
import type { NotifyOptions, ObserverFetchOptions } from './queryObserver'
import type { Query } from './query'

type InfiniteQueryObserverListener<TData, TError> = (
  result: InfiniteQueryObserverResult<TData, TError>,
) => void

export class InfiniteQueryObserver<
  TQueryFnData = unknown,
  TError = DefaultError,
  TData = InfiniteData<TQueryFnData>,
  TQueryData = TQueryFnData,
  TQueryKey extends QueryKey = QueryKey,
  TPageParam = unknown,
> extends QueryObserver<
  TQueryFnData,
  TError,
  TData,
  InfiniteData<TQueryData, TPageParam>,
  TQueryKey
> {
  // Type override
  subscribe!: Subscribable<
    InfiniteQueryObserverListener<TData, TError>
  >['subscribe']

  // Type override
  getCurrentResult!: () => InfiniteQueryObserverResult<TData, TError>

  // Type override
  protected fetch!: (
    fetchOptions: ObserverFetchOptions,
  ) => Promise<InfiniteQueryObserverResult<TData, TError>>

  constructor(
    client: QueryClient,
    options: InfiniteQueryObserverOptions<
      TQueryFnData,
      TError,
      TData,
      TQueryData,
      TQueryKey,
      TPageParam
    >,
  ) {
    super(client, options)
  }

  protected bindMethods(): void {
    super.bindMethods()

Review comment (suggestion):
It's trivial. but when doing type overriding, I want to use Subscribable and make it clearer to override while keeping the constraints of Subscribable, so I think it reduces the possibility of making constraint mistakes.

Code after:
import type { QueryClient } from './queryClient'
import type { NotifyOptions } from './queryObserver'
import type { Query } from './query'

type InfiniteQueryObserverListener<TData, TError> = (
  result: InfiniteQueryObserverResult<TData, TError>,
) => void

export class InfiniteQueryObserver<
  TQueryFnData = unknown,
  TError = DefaultError,
  TData = InfiniteData<TQueryFnData>,
  TQueryData = TQueryFnData,
  TQueryKey extends QueryKey = QueryKey,
  TPageParam = unknown,
> extends QueryObserver<
  TQueryFnData,
  TError,
  TData,
  InfiniteData<TQueryData, TPageParam>,
  TQueryKey
> {
  // Type override
  subscribe!: Subscribable<
    InfiniteQueryObserverListener<TData, TError>
  >['subscribe']

  // Type override
  getCurrentResult!: ReplaceReturnType<
    QueryObserver<
      TQueryFnData,
      TError,
      TData,
      InfiniteData<TQueryData, TPageParam>,
      TQueryKey
    >['getCurrentResult'],
    InfiniteQueryObserverResult<TData, TError>
  >

  // Type override
  protected fetch!: ReplaceReturnType<
    QueryObserver<
      TQueryFnData,
      TError,
      TData,
      InfiniteData<TQueryData, TPageParam>,
      TQueryKey
    >['fetch'],
    Promise<InfiniteQueryObserverResult<TData, TError>>
  >
Repository: tanstack/query
File: packages/query-core/src/__tests__/query.test.tsx (TypeScript)

Code before:

    queryClient.invalidateQueries({ queryKey: key })
    await sleep(1)
    queryClient.invalidateQueries({ queryKey: key })
    await sleep(1)
    unsubscribe() // unsubscribe to simulate unmount

    // set up a new observer to simulate a mount of new component
    const newObserver = new QueryObserver(queryClient, {
      queryKey: key,
      queryFn,
    })

    const spy = vi.fn()
    newObserver.subscribe(({ data }) => spy(data))
    await sleep(60) // let it resolve
    expect(spy).toHaveBeenCalledWith('1 - 2')
  })

  it('should have an error status when queryFn data is not serializable', async () => {
    const key = queryKey()

    const queryFn = vi.fn()

    queryFn.mockImplementation(async () => {
      await sleep(1000)

      const data: Array<{
        id: number
        name: string
        link: null | { id: number; name: string; link: unknown }
      }> = Array.from({ length: 5 })
        .fill(null)
        .map((_, index) => ({
          id: index,
          name: `name-${index}`,
          link: null,
        }))

      if (data[0] && data[1]) {
        data[0].link = data[1]
        data[1].link = data[0]
      }

      return data
    })

    await queryClient.prefetchQuery({ queryKey: key, queryFn })

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


Review comment (suggestion):
since we run with real timers:

```suggestion
      await sleep(10)
```

Code after:

    queryClient.invalidateQueries({ queryKey: key })
    await sleep(1)
    queryClient.invalidateQueries({ queryKey: key })
    await sleep(1)
    unsubscribe() // unsubscribe to simulate unmount

    // set up a new observer to simulate a mount of new component
    const newObserver = new QueryObserver(queryClient, {
      queryKey: key,
      queryFn,
    })

    const spy = vi.fn()
    newObserver.subscribe(({ data }) => spy(data))
    await sleep(60) // let it resolve
    expect(spy).toHaveBeenCalledWith('1 - 2')
  })

  it('should have an error status when queryFn data is not serializable', async () => {
    const consoleMock = vi.spyOn(console, 'error')

    consoleMock.mockImplementation(() => undefined)

    const key = queryKey()

    const queryFn = vi.fn()

    queryFn.mockImplementation(async () => {
      await sleep(10)

      const data: Array<{
        id: number
        name: string
        link: null | { id: number; name: string; link: unknown }
      }> = Array.from({ length: 5 })
        .fill(null)
        .map((_, index) => ({
          id: index,
          name: `name-${index}`,
          link: null,
        }))

      if (data[0] && data[1]) {
        data[0].link = data[1]
        data[1].link = data[0]
      }

      return data
    })
Repository: tanstack/query
File: packages/query-core/src/utils.ts (TypeScript)

Code before:
  }

  if (a && b && typeof a === 'object' && typeof b === 'object') {
    return !Object.keys(b).some((key) => !partialMatchKey(a[key], b[key]))
  }

  return false
}

/**
 * This function returns `a` if `b` is deeply equal.
 * If not, it will replace any deeply equal children of `b` with those of `a`.
 * This can be used for structural sharing between JSON values for example.
 */
export function replaceEqualDeep<T>(a: unknown, b: T): T
export function replaceEqualDeep(a: any, b: any): any {
  if (a === b) {
    return a
  }

  if (process.env.NODE_ENV !== 'production') {
    try {
      JSON.stringify(a)
      JSON.stringify(b)
    } catch {
      throw new Error('Values are not serializable')
    }
  }

  const array = isPlainArray(a) && isPlainArray(b)

  if (array || (isPlainObject(a) && isPlainObject(b))) {
    const aItems = array ? a : Object.keys(a)
    const aSize = aItems.length
    const bItems = array ? b : Object.keys(b)
    const bSize = bItems.length
    const copy: any = array ? [] : {}

    let equalItems = 0

    for (let i = 0; i < bSize; i++) {
      const key = array ? i : bItems[i]
      if (
        ((!array && aItems.includes(key)) || array) &&
        a[key] === undefined &&
        b[key] === undefined
      ) {
        copy[key] = undefined
        equalItems++
      } else {
        copy[key] = replaceEqualDeep(a[key], b[key])

Review comment (suggestion):
let's please add a good `console.error` log here, like we do in other places as well. Otherwise, it's up to the user to inspect this error and it might get unnoticed.

this likely means that the test will need to stub the console and assert that the log happened. We have some good examples of this already

Code after:
  }

  if (a && b && typeof a === 'object' && typeof b === 'object') {
    return !Object.keys(b).some((key) => !partialMatchKey(a[key], b[key]))
  }

  return false
}

/**
 * This function returns `a` if `b` is deeply equal.
 * If not, it will replace any deeply equal children of `b` with those of `a`.
 * This can be used for structural sharing between JSON values for example.
 */
export function replaceEqualDeep<T>(a: unknown, b: T): T
export function replaceEqualDeep(a: any, b: any): any {
  if (a === b) {
    return a
  }

  const array = isPlainArray(a) && isPlainArray(b)

  if (array || (isPlainObject(a) && isPlainObject(b))) {
    const aItems = array ? a : Object.keys(a)
    const aSize = aItems.length
    const bItems = array ? b : Object.keys(b)
    const bSize = bItems.length
    const copy: any = array ? [] : {}

    let equalItems = 0

    for (let i = 0; i < bSize; i++) {
      const key = array ? i : bItems[i]
      if (
        ((!array && aItems.includes(key)) || array) &&
        a[key] === undefined &&
        b[key] === undefined
      ) {
        copy[key] = undefined
        equalItems++
      } else {
        copy[key] = replaceEqualDeep(a[key], b[key])
        if (copy[key] === a[key] && a[key] !== undefined) {
          equalItems++
        }
      }
    }

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