feat: Preact Adapter #9935
feat: Preact Adapter #9935
Conversation
|
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Server
participant Client
participant HydrationBoundary
participant QueryClient
Note over Server, HydrationBoundary: Server renders and provides DehydratedState
Server->>Client: send HTML + dehydratedState
Client->>HydrationBoundary: mount with state prop
HydrationBoundary->>QueryClient: compare state.queries vs cache
opt New queries found
HydrationBoundary->>QueryClient: hydrate(newQueries) %% immediate during render
end
opt Existing queries present
HydrationBoundary->>HydrationBoundary: enqueue(existingQueries) %% defer
HydrationBoundary->>Client: render children
HydrationBoundary->>QueryClient: (useEffect) hydrate(enqueuedQueries) %% post-render
end
HydrationBoundary->>Client: children render completes
Note over Client,QueryClient: cache now contains hydrated queries
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
♻️ Duplicate comments (1)
packages/preact-query/src/__tests__/useInfiniteQuery.test.tsx (1)
2-3: Same React import inconsistency as other test files.This test file also uses React imports instead of Preact. See the comment on
usePrefetchQuery.test.tsxregarding consistency.
🟡 Minor comments (8)
packages/preact-query/CHANGELOG.md-1-1 (1)
1-1: Incorrect package name in changelog header.The header says
@tanstack/react-querybut this is thepreact-querypackage.-# @tanstack/react-query +# @tanstack/preact-querypackages/preact-query/src/__tests__/useQuery.test-d.tsx-1-5 (1)
1-5: MissingUseQueryResultimport causes type error.Line 283 references
UseQueryResultbut it's not imported. This will cause a TypeScript compilation error.import { describe, expectTypeOf, it } from 'vitest' import { queryKey } from '@tanstack/query-test-utils' import { useQuery } from '../useQuery' import { queryOptions } from '../queryOptions' -import type { OmitKeyof, QueryFunction, UseQueryOptions } from '..' +import type { OmitKeyof, QueryFunction, UseQueryOptions, UseQueryResult } from '..'examples/preact/simple/src/index.tsx-9-9 (1)
9-9: Addrel="noopener noreferrer"to external links withtarget="_blank".Using
target="_blank"withoutrel="noopener"exposes the page to potential security risks where the opened page can accesswindow.opener.- <a href="https://preactjs.com" target="_blank"> + <a href="https://preactjs.com" target="_blank" rel="noopener noreferrer">packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx-26-27 (1)
26-27: SharedqueryCacheandqueryClientmay cause cross-test pollution.Same issue as in the persister test file. Consider clearing the client in
afterEachor creating fresh instances per test.afterEach(() => { vi.useRealTimers() + queryClient.clear() })Committable suggestion skipped: line range outside the PR's diff.
packages/preact-query/src/__tests__/fine-grained-persister.test.tsx-20-21 (1)
20-21: SharedqueryCacheandqueryClientacross tests may cause test pollution.These instances are created once at module scope and reused across all tests. Without cleanup between tests (e.g.,
queryClient.clear()inafterEach), cached query state from one test can leak into another, causing flaky or false-positive results.+ afterEach(() => { + vi.useRealTimers() + queryClient.clear() + }) - afterEach(() => { - vi.useRealTimers() - })Alternatively, create fresh instances per test in
beforeEach.Committable suggestion skipped: line range outside the PR's diff.
examples/preact/simple/package.json-1-22 (1)
1-22: Update Vite to the latest 7.x patch version.Vite 7.0.4 (July 2025) is not the latest 7.x patch—newer versions (7.1/7.2 series) are available. Consider upgrading. TypeScript 5.9.3 (October 2025) and ESLint 9.36.0 (September 2025) are current valid releases.
packages/preact-query/src/__tests__/ssr.test.tsx-14-28 (1)
14-28:setIsServer(true)should be managed withinbeforeEach/afterEachlifecycle.
setIsServer(true)is called at the describe level (line 15), but its cleanup function is never invoked. This could leak the server state to other test suites.describe('Server Side Rendering', () => { - setIsServer(true) - let queryCache: QueryCache let queryClient: QueryClient + let restoreIsServer: () => void beforeEach(() => { + restoreIsServer = setIsServer(true) vi.useFakeTimers() queryCache = new QueryCache() queryClient = new QueryClient({ queryCache }) }) afterEach(() => { + restoreIsServer() vi.useRealTimers() })packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx-108-116 (1)
108-116: Bug:setQueryDatais passed the queryFn function instead of the query result.Line 111 sets
query.queryFn(the function itself) as the query data, rather than the resolved value. This test passes because the check isonSuspendnot being called, but the data shape is incorrect.Apply this diff to fix the test setup:
it('should not suspend on mount if query has been already fetched', () => { const query = createQuery(1) - queryClient.setQueryData(query.queryKey, query.queryFn) + queryClient.setQueryData(query.queryKey, 1) render(<TestComponent queries={[query]} />) expect(onSuspend).not.toHaveBeenCalled() })
🧹 Nitpick comments (39)
packages/preact-query/root.eslint.config.js (1)
20-40: Remove framework-specific terms not applicable to preact-query.The cspell word list contains terms specific to other TanStack Query adapters that don't apply to the Preact adapter:
solidjs(line 30) - SolidJS adaptervue-demi(line 37) - Vue adapterɵkind,ɵproviders(lines 38-39) - Angular adapterThis appears to be copied from another package's config. Consider removing these irrelevant entries and adding any Preact-specific terms if needed (e.g.,
preact).'tanstack', // Our package scope 'todos', // Too general word to be caught as error 'tsqd', // Our public interface (TanStack Query Devtools shorthand) 'tsup', // We use tsup as builder 'typecheck', // Field of vite.config.ts - 'vue-demi', // dependency of @tanstack/vue-query - 'ɵkind', // Angular specific - 'ɵproviders', // Angular specific + 'preact', // Our target framework ],packages/preact-query/src/HydrationBoundary.tsx (2)
21-21: Consider using Preact-native types instead of React types.
React.ReactNodeis used here, but this is a Preact package. While Preact's compatibility layer often allows React types to work, using Preact's nativeComponentChildrentype frompreactwould be more consistent.+import type { ComponentChildren } from 'preact' + export interface HydrationBoundaryProps { state: DehydratedState | null | undefined options?: OmitKeyof<HydrateOptions, 'defaultOptions'> & { defaultOptions?: OmitKeyof< Exclude<HydrateOptions['defaultOptions'], undefined>, 'mutations' > } - children?: React.ReactNode + children?: ComponentChildren queryClient?: QueryClient }
110-110: Consider using Preact's VNode type for the return cast.For consistency with the Preact ecosystem, consider using
VNodefrom Preact instead ofReact.ReactElement.+import type { VNode } from 'preact' + - return children as React.ReactElement + return children as VNodepackages/preact-query/src/__tests__/useQuery.promise.test.tsx (1)
20-37: Consider isolating QueryClient per test to prevent state leakage.The
queryClientandqueryCacheare shared across all tests in this file. WhilebeforeAll/afterAllhandle the experimental option, test pollution can occur if one test leaves stale cache entries. Consider either:
- Clearing the cache in an
afterEachhook, or- Creating a fresh
QueryClientper test+ afterEach(() => { + queryCache.clear() + })packages/preact-query/src/queryOptions.ts (1)
40-50: Consider usingOmitKeyoffor consistency.Line 45 uses plain
OmitwhileUnusedSkipTokenOptions(line 30) usesOmitKeyof. For consistency with the rest of the codebase and to leverage the additional type safety ofOmitKeyof, consider updating this.export type DefinedInitialDataOptions< TQueryFnData = unknown, TError = DefaultError, TData = TQueryFnData, TQueryKey extends QueryKey = QueryKey, -> = Omit<UseQueryOptions<TQueryFnData, TError, TData, TQueryKey>, 'queryFn'> & { +> = OmitKeyof<UseQueryOptions<TQueryFnData, TError, TData, TQueryKey>, 'queryFn'> & { initialData: | NonUndefinedGuard<TQueryFnData> | (() => NonUndefinedGuard<TQueryFnData>) queryFn?: QueryFunction<TQueryFnData, TQueryKey> }packages/preact-query/src/types.ts (1)
160-189: Inconsistent omit utilities between suspense result types.
UseSuspenseQueryResultusesDistributiveOmit(line 163) whileUseSuspenseInfiniteQueryResultusesOmitKeyof(line 186). Since both are applied to union types (DefinedQueryObserverResultandDefinedInfiniteQueryObserverResultare unions per core types), they should use the same utility for consistent behavior.
DistributiveOmitdistributes over unions, which is typically desired for result types.export type UseSuspenseInfiniteQueryResult< TData = unknown, TError = DefaultError, -> = OmitKeyof< +> = DistributiveOmit< DefinedInfiniteQueryObserverResult<TData, TError>, 'isPlaceholderData' | 'promise' >packages/preact-query/README.md (2)
5-29: Clarify that this package targets Preact, not React, in the READMELine 5 still advertises “Hooks for fetching, caching and updating asynchronous data in React”, which is potentially confusing for
@tanstack/preact-query. Consider updating the wording (and, if desired, the bundle-size badge and related links) to explicitly mention Preact so users landing on this README understand they’re in the Preact adapter package.
1-27: Optional: add meaningfulalttext for inline imagesA few
<img>tags use empty or missingaltattributes (e.g., Lines 1 and 12), which trips markdownlint and slightly hurts accessibility; adding short descriptions (oralt=""only where decorative) would address that.examples/preact/simple/tsconfig.json (1)
2-19: Verify thatpathsmappings are actually used by TypeScriptThis config defines
pathsfor"react"and"react-dom"(Lines 14–17) but doesn’t set abaseUrlorextendsanother config that does. In vanilla TypeScript,pathsare only honored whenbaseUrlis configured, so these aliases might be ignored by the language service even if Vite resolves them at build time. Consider adding"baseUrl": "."here or confirming that a parent tsconfig already provides it.packages/preact-query/src/__tests__/queryOptions.test.tsx (1)
5-13: Optional: decide whether identity or just structural equality is the contractRight now the test (Lines 6–13) uses
toStrictEqual, which only guarantees thatqueryOptionsreturns an equivalent object. If you want to lock in the stronger contract that it returns the same instance (matching the current implementation), you could switch totoBe(object); otherwise the existing assertion is fine and leaves room for cloning in the future.examples/preact/simple/README.md (1)
3-15: Minor docs/a11y polish for the example READMEYou might want to (a) add an
altattribute to the logo<img>on Line 4 and (b) wrap the bare URLs in Lines 11 and 15 in markdown links to satisfy markdownlint and slightly improve readability, but these are purely cosmetic.packages/preact-query/src/__tests__/infiniteQueryOptions.test.tsx (1)
15-15: UsetoBefor identity assertion.Since
infiniteQueryOptionsreturns the exact same object reference (identity behavior perinfiniteQueryOptions.tsline 146-148), usingtoBewould more accurately verify reference equality rather thantoStrictEqualwhich only checks deep equality.- expect(infiniteQueryOptions(object)).toStrictEqual(object) + expect(infiniteQueryOptions(object)).toBe(object)examples/preact/simple/src/index.tsx (1)
34-41: Add type annotations forResourceprops.Consider adding a TypeScript interface for the
Resourcecomponent props to improve type safety and developer experience.+interface ResourceProps { + title: string; + description: string; + href: string; +} + -function Resource(props) { +function Resource(props: ResourceProps) { return ( - <a href={props.href} target="_blank" class="resource"> + <a href={props.href} target="_blank" rel="noopener noreferrer" class="resource"> <h2>{props.title}</h2> <p>{props.description}</p> </a> ); }packages/preact-query/src/__tests__/mutationOptions.test.tsx (1)
10-18: Consider isolating QueryClient per test to prevent state leakage.A shared
QueryClientacross tests (created outside test blocks) can lead to flaky tests if mutations from previous tests persist in the cache. Consider creating a freshQueryClientin abeforeEachblock or within each test.describe('mutationOptions', () => { + let queryClient: QueryClient + beforeEach(() => { vi.useFakeTimers() + queryClient = new QueryClient() }) afterEach(() => { vi.useRealTimers() + queryClient.clear() })Note: Tests at lines 36-80 and beyond already create their own
queryClientwithin the test body, so this primarily affects consistency and future test additions.packages/preact-query/src/__tests__/useSuspenseInfiniteQuery.test.tsx (2)
12-14: Consider cleaning up QueryClient between tests.The shared
queryCacheandqueryClientat describe scope may accumulate mutation/query state across tests. Consider adding cleanup:describe('useSuspenseInfiniteQuery', () => { const queryCache = new QueryCache() const queryClient = new QueryClient({ queryCache }) + afterEach(() => { + queryCache.clear() + })
51-84: NODE_ENV manipulation is fragile but functional.The pattern of saving, modifying, and restoring
process.env.NODE_ENVworks in Vitest's runtime context. Consider usingvi.stubEnvfor cleaner environment stubbing that auto-restores:it('should log an error when skipToken is used in development environment', () => { - const envCopy = process.env.NODE_ENV - process.env.NODE_ENV = 'development' + vi.stubEnv('NODE_ENV', 'development') // ... test body ... consoleErrorSpy.mockRestore() - process.env.NODE_ENV = envCopy + vi.unstubAllEnvs() })With
restoreMocks: truein your Vite config, you could also rely on automatic restoration if usingvi.stubEnv.packages/preact-query/src/__tests__/fine-grained-persister.test.tsx (2)
2-2: Import should usepreact/compatinstead ofreactfor consistency.This is a Preact package, so the import should align with the package's ecosystem. While
preact/compatprovides React compatibility, explicitly importing fromreactin a Preact package test file may cause confusion and potential issues if the test environment isn't properly aliased.-import * as React from 'react' +import * as React from 'preact/compat'
54-67: Unused ref state pattern appears in all three test components.The
[_, setRef] = React.useState<HTMLDivElement | null>()andref={(value) => setRef(value)}pattern creates state that's never read. If this is intended to trigger re-renders on mount, consider documenting the intent or using a more explicit approach.Also applies to: 111-123, 152-164
examples/preact/simple/package.json (1)
12-18: Consider adding@tanstack/preact-query-devtoolsif available.If devtools are being introduced as part of this PR or planned, they would enhance the example. Otherwise, the dev dependencies look appropriate for the Vite + Preact + TypeScript stack.
packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx (1)
4-4: Import should usepreact/compatfor consistency with the Preact package.-import * as React from 'react' +import * as React from 'preact/compat'packages/preact-query/src/useIsFetching.ts (2)
8-10: TODO acknowledged: Consider addressing the preact/compat overhead.The comment is valid. If bundle size is a concern, using
@preact/signalsor a custom implementation ofuseSyncExternalStorecould reduce the preact/compat dependency overhead. This could be tracked as a follow-up optimization.Would you like me to open an issue to track this optimization?
19-26: Consider addinggetServerSnapshotfor SSR compatibility.
useSyncExternalStoreaccepts a third argument (getServerSnapshot) for server-side rendering. While the file is marked'use client', Preact apps may still encounter hydration scenarios where this is needed. ForisFetching, returning0during SSR is typically safe.return useSyncExternalStore( useCallback( (onStoreChange) => queryCache.subscribe(notifyManager.batchCalls(onStoreChange)), [queryCache], ), () => client.isFetching(filters), + () => 0, // Server snapshot: no queries fetching during SSR )Please verify if Preact's
useSyncExternalStorefrompreact/compatrequires the third argument for SSR scenarios in the TanStack Query use case.packages/preact-query/src/__tests__/suspense.test.tsx (1)
2-3: Imports should usepreact/compatfor consistency.-import { act, render } from '@testing-library/react' -import { Suspense } from 'react' +import { act, render } from '@testing-library/preact' +import { Suspense } from 'preact/compat'Note: Verify that
@testing-library/preactis available in the project's dev dependencies and provides equivalent functionality.packages/preact-query/src/__tests__/usePrefetchQuery.test.tsx (1)
26-36: Consider adding queryClient cleanup between tests.The
queryClientis created at module level but never cleared between tests. This can lead to test pollution where cached data from one test affects another.Add cleanup in
afterEach:afterEach(() => { vi.useRealTimers() + queryClient.clear() })packages/preact-query/src/QueryClientProvider.tsx (1)
3-5: Separate type imports from value imports.
ComponentChildrenandVNodeare types and should be imported usingimport typefor better tree-shaking and clarity.import type { QueryClient } from '@tanstack/query-core' -import { ComponentChildren, createContext, VNode } from 'preact' +import { createContext } from 'preact' +import type { ComponentChildren, VNode } from 'preact' import { useContext, useEffect } from 'preact/hooks'packages/preact-query/src/useMutation.ts (1)
42-44: Options object reference stability.The
optionsdependency will causesetOptionsto be called on every render sinceoptionsis typically a new object each time. This is acceptable becauseMutationObserver.setOptionsinternally handles diffing, but documenting this behavior or using a stable options reference pattern could improve clarity.packages/preact-query/src/__tests__/useInfiniteQuery.test.tsx (1)
49-65: Add queryClient cleanup between tests.Similar to the other test file, the shared
queryClientshould be cleared inafterEachto prevent test pollution.afterEach(() => { vi.useRealTimers() + queryClient.clear() })packages/preact-query/src/__tests__/useSuspenseQuery.test.tsx (2)
31-32: SharedqueryCacheandqueryClientmay cause test pollution.The
queryCacheandqueryClientare instantiated once at the module level and reused across all tests. This can lead to state leakage between tests if queries with the same key are used or if the cache isn't cleared properly.Consider moving these into
beforeEachto ensure test isolation:- const queryCache = new QueryCache() - const queryClient = new QueryClient({ queryCache }) + let queryCache: QueryCache + let queryClient: QueryClient beforeEach(() => { + queryCache = new QueryCache() + queryClient = new QueryClient({ queryCache }) vi.useFakeTimers() }) afterEach(() => { vi.useRealTimers() + queryClient.clear() })
665-667: Redundant error re-throw after suspense query.This explicit error re-throw is redundant for
useSuspenseQuerysince it already throws errors by default whenthrowOnErroris enabled (which is the default for suspense queries). This code path would only be reached after successful data resolution.- if (result.error) { - throw result.error - }packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx (2)
260-267: Timer setup duplication may cause conflicts.This nested
describeblock sets up fake timers withbeforeEach/afterEach, but the outerdescribeblock (lines 40-46) already usesbeforeAll/afterAllfor fake timers. This could lead to timer state conflicts between tests.Consider either:
- Removing the inner timer setup since the outer block already handles it, or
- Restructuring to have a single timer management strategy across all tests
670-677: Duplicate fake timer setup.This nested
describeblock duplicates the fake timer setup that already exists in the parentdescribeblock at lines 261-267. Since both usebeforeAll/afterAll, the inner setup is redundant.Remove the duplicate timer setup:
describe('gc (with fake timers)', () => { - beforeAll(() => { - vi.useFakeTimers() - }) - - afterAll(() => { - vi.useRealTimers() - }) - it('should gc when unmounted while fetching with low gcTime (#8159)', async () => {packages/preact-query/src/useBaseQuery.ts (1)
118-120: Effect runs on every render due todefaultedOptionsobject recreation.
defaultedOptionsis a new object on every render (created byclient.defaultQueryOptions(options)on line 59), causing thisuseEffectto run every render. Whileobserver.setOptionsmay internally short-circuit if options are equivalent, this is inefficient.Consider memoizing the options or using a ref-based comparison pattern similar to react-query's implementation to avoid unnecessary effect executions.
packages/preact-query/src/__tests__/utils.tsx (2)
9-23: Consider adding explicit return type.The
as anycast on line 22 bypasses type safety. Consider defining a proper return type that extends the render result with the custom rerender signature.+interface RenderWithClientResult extends Omit<ReturnType<typeof render>, 'rerender'> { + rerender: (rerenderUi: React.ReactElement) => void +} + export function renderWithClient( client: QueryClient, ui: React.ReactElement, -): ReturnType<typeof render> { +): RenderWithClientResult { const { rerender, ...result } = render( <QueryClientProvider client={client}>{ui}</QueryClientProvider>, ) return { ...result, rerender: (rerenderUi: React.ReactElement) => rerender( <QueryClientProvider client={client}>{rerenderUi}</QueryClientProvider>, ), - } as any + } }
59-72: Addconfigurable: trueto prevent errors on repeated calls.If
setIsServeris called multiple times without restoring first, the secondObject.definePropertycall will fail because the property is not configurable by default.export function setIsServer(isServer: boolean) { const original = utils.isServer Object.defineProperty(utils, 'isServer', { get: () => isServer, + configurable: true, }) return () => { Object.defineProperty(utils, 'isServer', { get: () => original, + configurable: true, }) } }packages/preact-query/src/__tests__/useQueries.test.tsx (1)
40-41: SharedqueryClientacross tests may cause test pollution.The
queryCacheandqueryClientare created once at module scope and shared across all tests. While some tests create their own client, others reuse this shared instance. Consider clearing the cache inafterEachto ensure test isolation:afterEach(() => { vi.useRealTimers() + queryCache.clear() })Alternatively, create a fresh client per test using a
beforeEachhook.packages/preact-query/src/useMutationState.ts (2)
14-16: Acknowledge the TODO for bundle optimization.The TODO comment correctly identifies that
preact/compatadds overhead. Consider tracking this as a follow-up issue to implement a lighter-weight subscription mechanism using Preact's native signals or a custom store abstraction.Would you like me to open an issue to track this optimization opportunity?
52-55: Consider initializingresultref directly to avoid non-null assertion.The ref is typed as potentially
nullbut immediately initialized. A cleaner pattern would avoid the assertion:- const result = useRef<Array<TResult>>(null) - if (result.current === null) { - result.current = getResult(mutationCache, options) - } + const result = useRef<Array<TResult>>(getResult(mutationCache, options))This eliminates the need for the
!assertion on line 77 and makes the initialization intent clearer.Also applies to: 76-77
packages/preact-query/src/useQueries.ts (1)
249-252: Side effects during render may cause issues with concurrent rendering.Calling
ensureSuspenseTimersandensurePreventErrorBoundaryRetrydirectly during render (outside of hooks) can trigger unexpected behavior in concurrent mode or strict mode, where the render phase may execute multiple times. These mutations should be applied within theuseMemocallback wheredefaultedQueriesis computed, or the options should be cloned before mutation.Consider moving these calls inside the
useMemo:const defaultedQueries = useMemo( () => queries.map((opts) => { const defaultedOptions = client.defaultQueryOptions( opts as QueryObserverOptions, ) // Make sure the results are already in fetching state before subscribing or updating options defaultedOptions._optimisticResults = isRestoring ? 'isRestoring' : 'optimistic' + ensureSuspenseTimers(defaultedOptions) + ensurePreventErrorBoundaryRetry(defaultedOptions, errorResetBoundary) + return defaultedOptions }), - [queries, client, isRestoring], + [queries, client, isRestoring, errorResetBoundary], ) -defaultedQueries.forEach((query) => { - ensureSuspenseTimers(query) - ensurePreventErrorBoundaryRetry(query, errorResetBoundary) -})packages/preact-query/src/useSuspenseQueries.ts (1)
165-187: Redundant function overloads.The two overload signatures (lines 165-176 and 178-187) appear to be identical in their
queriestype definition. Consider consolidating into a single overload to reduce complexity.export function useSuspenseQueries< T extends Array<any>, TCombinedResult = SuspenseQueriesResults<T>, >( options: { queries: | readonly [...SuspenseQueriesOptions<T>] | readonly [...{ [K in keyof T]: GetUseSuspenseQueryOptions<T[K]> }] combine?: (result: SuspenseQueriesResults<T>) => TCombinedResult }, queryClient?: QueryClient, ): TCombinedResult - -export function useSuspenseQueries< - T extends Array<any>, - TCombinedResult = SuspenseQueriesResults<T>, ->( - options: { - queries: readonly [...SuspenseQueriesOptions<T>] - combine?: (result: SuspenseQueriesResults<T>) => TCombinedResult - }, - queryClient?: QueryClient, -): TCombinedResult
| // @ts-expect-error wtf | ||
| ...reactHooks.configs['recommended-latest'], |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace unprofessional comment with a descriptive explanation.
The @ts-expect-error wtf comment should be replaced with a meaningful explanation of why the type error occurs and is expected.
- // @ts-expect-error wtf
+ // @ts-expect-error types are not correctly exported from eslint-plugin-react-hooks
...reactHooks.configs['recommended-latest'],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // @ts-expect-error wtf | |
| ...reactHooks.configs['recommended-latest'], | |
| // @ts-expect-error types are not correctly exported from eslint-plugin-react-hooks | |
| ...reactHooks.configs['recommended-latest'], |
🤖 Prompt for AI Agents
In packages/preact-query/eslint.config.js around lines 11 to 12, replace the
unprofessional comment "// @ts-expect-error wtf" with a concise, descriptive
explanation that documents why a TypeScript type error is expected (for example:
mismatch between ESLint config types and reactHooks.configs shape, or upstream
type definitions incompatible with this project's TS version), keep the
@ts-expect-error directive if needed, and include the ticket/issue reference or
link and the conditions under which it can be removed so future maintainers
understand the rationale.
packages/preact-query/src/__tests__/QueryClientProvider.test.tsx
Outdated
Show resolved
Hide resolved
| export type DefinedInitialDataInfiniteOptions< | ||
| TQueryFnData, | ||
| TError = DefaultError, | ||
| TData = InfiniteData<TQueryFnData>, | ||
| TQueryKey extends QueryKey = QueryKey, | ||
| TPageParam = unknown, | ||
| > = UseInfiniteQueryOptions< | ||
| TQueryFnData, | ||
| TError, | ||
| TData, | ||
| TQueryKey, | ||
| TPageParam | ||
| > & { | ||
| initialData: | ||
| | NonUndefinedGuard<InfiniteData<TQueryFnData, TPageParam>> | ||
| | (() => NonUndefinedGuard<InfiniteData<TQueryFnData, TPageParam>>) | ||
| | undefined | ||
| } |
There was a problem hiding this comment.
DefinedInitialDataInfiniteOptions should not allow undefined in initialData.
The type name suggests that initialData is always defined, but line 72 includes | undefined in the union, which contradicts the intent. Compare with the non-infinite DefinedInitialDataOptions in queryOptions.ts (lines 46-48) which correctly omits undefined.
export type DefinedInitialDataInfiniteOptions<
TQueryFnData,
TError = DefaultError,
TData = InfiniteData<TQueryFnData>,
TQueryKey extends QueryKey = QueryKey,
TPageParam = unknown,
> = UseInfiniteQueryOptions<
TQueryFnData,
TError,
TData,
TQueryKey,
TPageParam
> & {
initialData:
| NonUndefinedGuard<InfiniteData<TQueryFnData, TPageParam>>
| (() => NonUndefinedGuard<InfiniteData<TQueryFnData, TPageParam>>)
- | undefined
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type DefinedInitialDataInfiniteOptions< | |
| TQueryFnData, | |
| TError = DefaultError, | |
| TData = InfiniteData<TQueryFnData>, | |
| TQueryKey extends QueryKey = QueryKey, | |
| TPageParam = unknown, | |
| > = UseInfiniteQueryOptions< | |
| TQueryFnData, | |
| TError, | |
| TData, | |
| TQueryKey, | |
| TPageParam | |
| > & { | |
| initialData: | |
| | NonUndefinedGuard<InfiniteData<TQueryFnData, TPageParam>> | |
| | (() => NonUndefinedGuard<InfiniteData<TQueryFnData, TPageParam>>) | |
| | undefined | |
| } | |
| export type DefinedInitialDataInfiniteOptions< | |
| TQueryFnData, | |
| TError = DefaultError, | |
| TData = InfiniteData<TQueryFnData>, | |
| TQueryKey extends QueryKey = QueryKey, | |
| TPageParam = unknown, | |
| > = UseInfiniteQueryOptions< | |
| TQueryFnData, | |
| TError, | |
| TData, | |
| TQueryKey, | |
| TPageParam | |
| > & { | |
| initialData: | |
| | NonUndefinedGuard<InfiniteData<TQueryFnData, TPageParam>> | |
| | (() => NonUndefinedGuard<InfiniteData<TQueryFnData, TPageParam>>) | |
| } |
🤖 Prompt for AI Agents
In packages/preact-query/src/infiniteQueryOptions.ts around lines 56 to 73, the
DefinedInitialDataInfiniteOptions type currently allows initialData to be
undefined, which contradicts the type name; remove the trailing " | undefined"
from the initialData union so initialData is strictly
NonUndefinedGuard<InfiniteData<...>> or a function returning that type, matching
the non-infinite DefinedInitialDataOptions pattern.
| useSyncExternalStore( | ||
| useCallback( | ||
| (onStoreChange) => { | ||
| const unsubscribe = shouldSubscribe | ||
| ? observer.subscribe(notifyManager.batchCalls(onStoreChange)) | ||
| : noop | ||
|
|
||
| // Update result to make sure we did not miss any query updates | ||
| // between creating the observer and subscribing to it. | ||
| observer.updateResult() | ||
|
|
||
| return unsubscribe | ||
| }, | ||
| [observer, shouldSubscribe], | ||
| ), | ||
| () => observer.getCurrentResult(), | ||
| ) |
There was a problem hiding this comment.
Missing getServerSnapshot parameter may cause SSR hydration issues.
useSyncExternalStore requires a third argument (getServerSnapshot) for proper SSR support. Without it, server-side rendering will either fail or produce hydration mismatches when the server and client snapshots differ.
Apply this diff to add SSR support:
useSyncExternalStore(
useCallback(
(onStoreChange) => {
const unsubscribe = shouldSubscribe
? observer.subscribe(notifyManager.batchCalls(onStoreChange))
: noop
// Update result to make sure we did not miss any query updates
// between creating the observer and subscribing to it.
observer.updateResult()
return unsubscribe
},
[observer, shouldSubscribe],
),
() => observer.getCurrentResult(),
+ () => observer.getCurrentResult(),
)Alternatively, if SSR behavior should differ (e.g., return optimistic result), adjust accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useSyncExternalStore( | |
| useCallback( | |
| (onStoreChange) => { | |
| const unsubscribe = shouldSubscribe | |
| ? observer.subscribe(notifyManager.batchCalls(onStoreChange)) | |
| : noop | |
| // Update result to make sure we did not miss any query updates | |
| // between creating the observer and subscribing to it. | |
| observer.updateResult() | |
| return unsubscribe | |
| }, | |
| [observer, shouldSubscribe], | |
| ), | |
| () => observer.getCurrentResult(), | |
| ) | |
| useSyncExternalStore( | |
| useCallback( | |
| (onStoreChange) => { | |
| const unsubscribe = shouldSubscribe | |
| ? observer.subscribe(notifyManager.batchCalls(onStoreChange)) | |
| : noop | |
| // Update result to make sure we did not miss any query updates | |
| // between creating the observer and subscribing to it. | |
| observer.updateResult() | |
| return unsubscribe | |
| }, | |
| [observer, shouldSubscribe], | |
| ), | |
| () => observer.getCurrentResult(), | |
| () => observer.getCurrentResult(), | |
| ) |
🤖 Prompt for AI Agents
In packages/preact-query/src/useBaseQuery.ts around lines 100–116,
useSyncExternalStore is called without the third getServerSnapshot argument
which can cause SSR/hydration mismatches; fix by passing a getServerSnapshot
function appropriate for SSR (for example, a function that returns
observer.getCurrentResult() or the observer's optimistic/server snapshot method
if available) as the third parameter so the server snapshot is stable during
hydration; ensure the getServerSnapshot is a no-arg function (not invoked) and
aligns with the client snapshot behavior used in the second argument.
| const result = useSyncExternalStore( | ||
| useCallback( | ||
| (onStoreChange) => | ||
| observer.subscribe(notifyManager.batchCalls(onStoreChange)), | ||
| [observer], | ||
| ), | ||
| () => observer.getCurrentResult(), | ||
| ) |
There was a problem hiding this comment.
Missing getServerSnapshot argument for SSR support.
useSyncExternalStore requires a third argument (getServerSnapshot) for SSR compatibility. Without it, server-side rendering will fail or produce inconsistent results.
const result = useSyncExternalStore(
useCallback(
(onStoreChange) =>
observer.subscribe(notifyManager.batchCalls(onStoreChange)),
[observer],
),
() => observer.getCurrentResult(),
+ () => observer.getCurrentResult(),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const result = useSyncExternalStore( | |
| useCallback( | |
| (onStoreChange) => | |
| observer.subscribe(notifyManager.batchCalls(onStoreChange)), | |
| [observer], | |
| ), | |
| () => observer.getCurrentResult(), | |
| ) | |
| const result = useSyncExternalStore( | |
| useCallback( | |
| (onStoreChange) => | |
| observer.subscribe(notifyManager.batchCalls(onStoreChange)), | |
| [observer], | |
| ), | |
| () => observer.getCurrentResult(), | |
| () => observer.getCurrentResult(), | |
| ) |
🤖 Prompt for AI Agents
In packages/preact-query/src/useMutation.ts around lines 46 to 53, the
useSyncExternalStore call is missing the third getServerSnapshot argument
required for SSR compatibility; add a third parameter that returns the current
observer result (for example, a function that calls observer.getCurrentResult())
so server renders use the same snapshot as the client — ensure the function
reads the observer synchronously (e.g., () => observer.getCurrentResult()) to
provide a stable server snapshot.
| useSyncExternalStore( | ||
| useCallback( | ||
| (onStoreChange) => | ||
| shouldSubscribe | ||
| ? observer.subscribe(notifyManager.batchCalls(onStoreChange)) | ||
| : noop, | ||
| [observer, shouldSubscribe], | ||
| ), | ||
| () => observer.getCurrentResult(), | ||
| ) |
There was a problem hiding this comment.
Missing getServerSnapshot for SSR compatibility.
The useSyncExternalStore hook is called without the third argument (getServerSnapshot), which is required for proper SSR/hydration support. Without it, server rendering may fail or produce hydration mismatches.
useSyncExternalStore(
useCallback(
(onStoreChange) =>
shouldSubscribe
? observer.subscribe(notifyManager.batchCalls(onStoreChange))
: noop,
[observer, shouldSubscribe],
),
() => observer.getCurrentResult(),
+ () => observer.getCurrentResult(),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useSyncExternalStore( | |
| useCallback( | |
| (onStoreChange) => | |
| shouldSubscribe | |
| ? observer.subscribe(notifyManager.batchCalls(onStoreChange)) | |
| : noop, | |
| [observer, shouldSubscribe], | |
| ), | |
| () => observer.getCurrentResult(), | |
| ) | |
| useSyncExternalStore( | |
| useCallback( | |
| (onStoreChange) => | |
| shouldSubscribe | |
| ? observer.subscribe(notifyManager.batchCalls(onStoreChange)) | |
| : noop, | |
| [observer, shouldSubscribe], | |
| ), | |
| () => observer.getCurrentResult(), | |
| () => observer.getCurrentResult(), | |
| ) |
🤖 Prompt for AI Agents
In packages/preact-query/src/useQueries.ts around lines 273 to 282,
useSyncExternalStore is being called without the required third argument for
server snapshots; add a getServerSnapshot function (e.g., a function that
returns observer.getCurrentResult() or an appropriate server-side result) as the
third parameter to ensure SSR/hydration compatibility, passing the same logic
used in the client snapshot but returning a stable server value or undefined
when server data isn’t available.
| @@ -0,0 +1,76 @@ | |||
| 'use client' | |||
There was a problem hiding this comment.
FWIW, no need for these as they do nothing in Preact. None of our tooling acknowledges these directives.
There was a problem hiding this comment.
It's certainly not a problem, no need for an apology. Just wanted to drop mention in case you weren't aware is all.
There was a problem hiding this comment.
yo @rschristian I had some questions and would be great to get a fresh set of eyes on some of these tests. Can I reach out to you on discord or any other platform? totally cool if not; I understand it's holiday season
There was a problem hiding this comment.
I don't use Discord, but you can DM me on Twitter or Bluesky (both linked from my profile here) or the Preact Slack, whichever is convenient. Will answer when I can
| 'mutations' | ||
| > | ||
| } | ||
| children?: React.ReactNode |
There was a problem hiding this comment.
You'd want ComponentChildren from preact
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/preact-query/src/HydrationBoundary.tsx (1)
95-96: Update the ESLint disable comment for Preact context.The comment references
react-hooks/refs, but this is a Preact project. Consider updating the comment to reference Preact's hooks rules or provide a clearer explanation of why the side effect inuseMemois intentional.- // eslint-disable-next-line react-hooks/refs + // Intentionally calling hydrate() during render for SSR and transition safety. + // This ensures new queries are available before children render. hydrate(client, { queries: newQueries }, optionsRef.current)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/preact-query/src/HydrationBoundary.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: TkDodo
Repo: TanStack/query PR: 9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.184Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
🧬 Code graph analysis (1)
packages/preact-query/src/HydrationBoundary.tsx (3)
packages/preact-query/src/index.ts (3)
HydrationBoundaryProps(40-40)HydrationBoundary(39-39)useQueryClient(35-35)packages/query-core/src/types.ts (1)
OmitKeyof(19-29)packages/preact-query/src/QueryClientProvider.tsx (1)
useQueryClient(11-23)
🔇 Additional comments (4)
packages/preact-query/src/HydrationBoundary.tsx (4)
14-24: LGTM!The interface is well-structured with appropriate type constraints. The exclusion of
'mutations'fromdefaultOptionsappears intentional for hydration-specific configuration.
26-37: LGTM!The ref-based options tracking correctly avoids triggering re-hydration when options change, while ensuring the latest options are available during hydration callbacks.
105-109: LGTM!The deferred hydration effect correctly handles existing queries post-render, preventing premature updates during transitions as explained in the component's documentation.
111-111: LGTM!The explicit type cast ensures type safety for the return value, handling the optional
childrenprop appropriately.
|
Just wanted to note my appreciation for this, as a member of a team using Tanstack Query with preact 🫶 |
thank you 😭❤️ |
…ry custom implementation)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/preact-query/src/__tests__/usePrefetchInfiniteQuery.test.tsx (1)
1-14: LGTM! Preact imports are correct.The file correctly imports from
@testing-library/preact,preact/compat, andpreact. The past review comments about React imports appear to have been addressed.packages/preact-query/src/__tests__/useQueries.test.tsx (1)
1-29: LGTM! Preact imports are correct.The file correctly imports from
@testing-library/preactandpreact/hooks. The past review comments about React imports appear to have been addressed.
🧹 Nitpick comments (4)
packages/preact-query/src/__tests__/useSuspenseInfiniteQuery.test.tsx (1)
16-49: Consider clarifying the always-true condition.Line 29 uses
Math.random() >= 0which is always true. While this appears intentional to bypass TypeScript type checking while still testing skipToken at runtime, it could be confusing to maintainers.Consider adding a comment to clarify the intent:
queryFn: Math.random() >= 0 ? skipToken : () => Promise.resolve(5), + // Always true - used to bypass TS checking while testing skipToken at runtimeOr use a more explicit pattern:
- queryFn: Math.random() >= 0 ? skipToken : () => Promise.resolve(5), + queryFn: (true as boolean) ? skipToken : () => Promise.resolve(5),packages/preact-query/src/__tests__/useMutation.test.tsx (1)
3-3: Use Preact hooks instead of React.This Preact adapter test file imports
Reactand usesReact.useEffect/React.useStatethroughout (lines 282, 332, 386, 418, 678, 855). For consistency with the package's purpose, use Preact hooks directly.Apply this diff:
-import * as React from 'react' +import { useEffect, useState } from 'preact/hooks'Then replace all
React.useEffectwithuseEffectandReact.useStatewithuseStatethroughout the file.packages/preact-query/src/__tests__/HydrationBoundary.test.tsx (1)
15-15: Consider renaming describe block to "Preact hydration".The describe block is named
'React hydration'but this is testing Preact's HydrationBoundary. Consider updating for consistency with the package.-describe('React hydration', () => { +describe('Preact hydration', () => {packages/preact-query/src/__tests__/useSuspenseQuery.test.tsx (1)
31-32: Consider moving queryClient/queryCache creation into beforeEach for test isolation.The
queryCacheandqueryClientare created at module level, which means they persist across all tests. While some tests callqueryClient.clear(), this pattern can lead to test pollution if a test fails before cleanup.- const queryCache = new QueryCache() - const queryClient = new QueryClient({ queryCache }) + let queryCache: QueryCache + let queryClient: QueryClient + beforeEach(() => { + queryCache = new QueryCache() + queryClient = new QueryClient({ queryCache }) + }) + + afterEach(() => { + queryClient.clear() + })Then remove the individual
queryClient.clear()calls from tests, or keep them for explicit cleanup within specific tests.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
packages/preact-query/eslint.config.js(1 hunks)packages/preact-query/package.json(1 hunks)packages/preact-query/src/__tests__/HydrationBoundary.test.tsx(1 hunks)packages/preact-query/src/__tests__/QueryClientProvider.test.tsx(1 hunks)packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx(1 hunks)packages/preact-query/src/__tests__/fine-grained-persister.test.tsx(1 hunks)packages/preact-query/src/__tests__/mutationOptions.test.tsx(1 hunks)packages/preact-query/src/__tests__/ssr-hydration.test.tsx(1 hunks)packages/preact-query/src/__tests__/ssr.test.tsx(1 hunks)packages/preact-query/src/__tests__/suspense.test.tsx(1 hunks)packages/preact-query/src/__tests__/useInfiniteQuery.test.tsx(1 hunks)packages/preact-query/src/__tests__/useIsFetching.test.tsx(1 hunks)packages/preact-query/src/__tests__/useMutation.test.tsx(1 hunks)packages/preact-query/src/__tests__/useMutationState.test.tsx(1 hunks)packages/preact-query/src/__tests__/usePrefetchInfiniteQuery.test.tsx(1 hunks)packages/preact-query/src/__tests__/usePrefetchQuery.test.tsx(1 hunks)packages/preact-query/src/__tests__/useQueries.test.tsx(1 hunks)packages/preact-query/src/__tests__/useQuery.promise.test.tsx(1 hunks)packages/preact-query/src/__tests__/useSuspenseInfiniteQuery.test.tsx(1 hunks)packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx(1 hunks)packages/preact-query/src/__tests__/useSuspenseQuery.test.tsx(1 hunks)packages/preact-query/src/__tests__/utils.tsx(1 hunks)packages/preact-query/test-setup.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/preact-query/src/tests/useSuspenseQueries.test.tsx
- packages/preact-query/src/tests/fine-grained-persister.test.tsx
- packages/preact-query/src/tests/ssr.test.tsx
- packages/preact-query/src/tests/usePrefetchQuery.test.tsx
- packages/preact-query/test-setup.ts
- packages/preact-query/eslint.config.js
- packages/preact-query/src/tests/ssr-hydration.test.tsx
- packages/preact-query/package.json
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: TkDodo
Repo: TanStack/query PR: 9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.184Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
📚 Learning: 2025-11-22T09:06:05.219Z
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
Applied to files:
packages/preact-query/src/__tests__/mutationOptions.test.tsxpackages/preact-query/src/__tests__/useInfiniteQuery.test.tsxpackages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsxpackages/preact-query/src/__tests__/useMutation.test.tsxpackages/preact-query/src/__tests__/QueryClientProvider.test.tsxpackages/preact-query/src/__tests__/HydrationBoundary.test.tsxpackages/preact-query/src/__tests__/usePrefetchInfiniteQuery.test.tsxpackages/preact-query/src/__tests__/useSuspenseInfiniteQuery.test.tsxpackages/preact-query/src/__tests__/useSuspenseQuery.test.tsxpackages/preact-query/src/__tests__/useQuery.promise.test.tsxpackages/preact-query/src/__tests__/useMutationState.test.tsxpackages/preact-query/src/__tests__/useIsFetching.test.tsxpackages/preact-query/src/__tests__/suspense.test.tsxpackages/preact-query/src/__tests__/useQueries.test.tsx
📚 Learning: 2025-11-02T22:52:33.071Z
Learnt from: DogPawHat
Repo: TanStack/query PR: 9835
File: packages/query-core/src/__tests__/queryClient.test-d.tsx:242-256
Timestamp: 2025-11-02T22:52:33.071Z
Learning: In the TanStack Query codebase, the new `query` and `infiniteQuery` methods support the `select` option for data transformation, while the legacy `fetchQuery` and `fetchInfiniteQuery` methods do not support `select` and should reject it at the type level.
Applied to files:
packages/preact-query/src/__tests__/useInfiniteQuery.test.tsxpackages/preact-query/src/__tests__/usePrefetchInfiniteQuery.test.tsxpackages/preact-query/src/__tests__/useQueries.test.tsx
📚 Learning: 2025-08-19T03:18:18.303Z
Learnt from: oscartbeaumont
Repo: TanStack/query PR: 9564
File: packages/solid-query-devtools/src/production.tsx:2-3
Timestamp: 2025-08-19T03:18:18.303Z
Learning: In the solid-query-devtools package, the codebase uses a pattern of type-only default imports combined with typeof for component type annotations (e.g., `import type SolidQueryDevtoolsComp from './devtools'` followed by `typeof SolidQueryDevtoolsComp`). This pattern is consistently used across index.tsx and production.tsx files, and the maintainers prefer consistency over changing this approach.
Applied to files:
packages/preact-query/src/__tests__/QueryClientProvider.test.tsxpackages/preact-query/src/__tests__/HydrationBoundary.test.tsxpackages/preact-query/src/__tests__/usePrefetchInfiniteQuery.test.tsxpackages/preact-query/src/__tests__/useMutationState.test.tsxpackages/preact-query/src/__tests__/useIsFetching.test.tsxpackages/preact-query/src/__tests__/useQueries.test.tsx
📚 Learning: 2025-09-02T17:57:33.184Z
Learnt from: TkDodo
Repo: TanStack/query PR: 9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.184Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
Applied to files:
packages/preact-query/src/__tests__/HydrationBoundary.test.tsxpackages/preact-query/src/__tests__/useQueries.test.tsx
🧬 Code graph analysis (9)
packages/preact-query/src/__tests__/useMutation.test.tsx (6)
packages/preact-query/src/index.ts (1)
useMutation(53-53)packages/preact-query/src/useMutation.ts (1)
useMutation(23-72)packages/query-core/src/mutationObserver.ts (2)
mutate(128-143)state(145-159)packages/preact-query/src/__tests__/utils.tsx (4)
renderWithClient(11-25)setActTimeout(53-59)mockOnlineManagerIsOnline(47-51)ErrorBoundary(80-114)packages/query-core/src/queriesObserver.ts (1)
result(195-210)packages/preact-query/src/types.ts (1)
UseMutationResult(237-242)
packages/preact-query/src/__tests__/QueryClientProvider.test.tsx (3)
packages/preact-query/src/index.ts (3)
useQuery(10-10)QueryClientProvider(34-34)useQueryClient(35-35)packages/preact-query/src/useQuery.ts (1)
useQuery(50-52)packages/preact-query/src/QueryClientProvider.tsx (2)
QueryClientProvider(30-46)useQueryClient(11-23)
packages/preact-query/src/__tests__/HydrationBoundary.test.tsx (3)
packages/preact-query/src/useQuery.ts (1)
useQuery(50-52)packages/preact-query/src/QueryClientProvider.tsx (1)
QueryClientProvider(30-46)packages/preact-query/src/HydrationBoundary.tsx (1)
HydrationBoundary(26-112)
packages/preact-query/src/__tests__/usePrefetchInfiniteQuery.test.tsx (5)
packages/query-core/src/types.ts (1)
InfiniteData(204-207)packages/preact-query/src/index.ts (2)
useSuspenseInfiniteQuery(12-12)usePrefetchInfiniteQuery(19-19)packages/preact-query/src/useSuspenseInfiniteQuery.ts (1)
useSuspenseInfiniteQuery(18-50)packages/preact-query/src/usePrefetchInfiniteQuery.tsx (1)
usePrefetchInfiniteQuery(9-30)packages/preact-query/src/__tests__/utils.tsx (1)
renderWithClient(11-25)
packages/preact-query/src/__tests__/useSuspenseInfiniteQuery.test.tsx (3)
packages/preact-query/src/index.ts (1)
useSuspenseInfiniteQuery(12-12)packages/preact-query/src/useSuspenseInfiniteQuery.ts (1)
useSuspenseInfiniteQuery(18-50)packages/preact-query/src/__tests__/utils.tsx (1)
renderWithClient(11-25)
packages/preact-query/src/__tests__/useMutationState.test.tsx (2)
packages/preact-query/src/useMutationState.ts (2)
useIsMutating(18-27)useMutationState(46-78)packages/preact-query/src/__tests__/utils.tsx (1)
renderWithClient(11-25)
packages/preact-query/src/__tests__/suspense.test.tsx (5)
packages/preact-query/src/QueryClientProvider.tsx (1)
QueryClientProvider(30-46)packages/preact-query/src/index.ts (2)
QueryClientProvider(34-34)useSuspenseQuery(11-11)packages/query-core/src/queryObserver.ts (1)
options(379-385)packages/query-core/src/types.ts (1)
QueryKey(53-61)packages/preact-query/src/useSuspenseQuery.ts (1)
useSuspenseQuery(8-34)
packages/preact-query/src/__tests__/useQueries.test.tsx (5)
packages/query-core/src/queriesObserver.ts (2)
result(195-210)queries(235-268)packages/preact-query/src/index.ts (2)
useQueries(8-8)queryOptions(20-20)packages/preact-query/src/useQueries.ts (1)
useQueries(212-335)packages/preact-query/src/__tests__/utils.tsx (1)
renderWithClient(11-25)packages/preact-query/src/queryOptions.ts (1)
queryOptions(85-87)
packages/preact-query/src/__tests__/utils.tsx (3)
packages/query-core/src/queriesObserver.ts (1)
result(195-210)packages/preact-query/src/QueryClientProvider.tsx (1)
QueryClientProvider(30-46)packages/query-core/src/timeoutManager.ts (2)
clearTimeout(108-110)setTimeout(101-106)
🔇 Additional comments (17)
packages/preact-query/src/__tests__/mutationOptions.test.tsx (1)
1-526: LGTM! Comprehensive test coverage for mutationOptions.The test suite thoroughly validates mutationOptions' identity behavior and integration with useIsMutating, useMutation, useMutationState, and queryClient.isMutating across various scenarios (with/without mutationKey, filtering, concurrent mutations).
packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx (1)
1-867: LGTM! Comprehensive QueryErrorResetBoundary test coverage.The test suite thoroughly validates error boundary interactions with useQuery, useQueries, useSuspenseQuery, and useSuspenseQueries across various scenarios including boundary resets, disabled queries, initialData, manual refetch, and multiple reset cycles.
packages/preact-query/src/__tests__/useInfiniteQuery.test.tsx (1)
1-1860: LGTM! Extensive useInfiniteQuery test coverage.The test suite comprehensively validates useInfiniteQuery behavior including:
- Initial and subsequent state transitions
- Error handling for fetchNextPage, fetchPreviousPage, and refetch
- PlaceholderData and keepPreviousData behavior
- Data selection and transformation
- Cancellation semantics
- Integration with queryClient operations
- Suspense flows with React.use()
- Edge cases with initialData, hasNextPage calculations, and cursor rebuilding
packages/preact-query/src/__tests__/utils.tsx (1)
1-114: LGTM! Well-structured test utilities.The test utilities provide essential helpers for the test suite:
renderWithClient: Consistent QueryClientProvider wrappingBlink: Controlled visibility toggling for timing testsmockOnlineManagerIsOnline: Online/offline simulationsetActTimeout: act()-wrapped setTimeout for React updatessetIsServer: Environment simulationErrorBoundary: Custom error boundary for boundary-driven retry testspackages/preact-query/src/__tests__/useMutationState.test.tsx (1)
1-238: LGTM! Comprehensive useMutationState test coverage.The test suite thoroughly validates useIsMutating and useMutationState behavior including:
- Tracking concurrent mutations
- Filtering by mutationKey and predicate
- Custom QueryClient usage
- Mutation variables and state transitions
The imports correctly use Preact testing utilities.
packages/preact-query/src/__tests__/useMutation.test.tsx (1)
1-1182: Comprehensive test coverage for useMutation.The test suite thoroughly covers mutation lifecycle, callback hooks, retry logic, online/offline handling, error boundaries, and observer concurrency. The test patterns follow TanStack Query conventions well.
packages/preact-query/src/__tests__/QueryClientProvider.test.tsx (1)
1-165: LGTM!The test file correctly uses
@testing-library/preactand covers the essential QueryClientProvider scenarios: cache assignment, cache partitioning, default options propagation, and error handling when no client is provided.packages/preact-query/src/__tests__/useIsFetching.test.tsx (1)
1-246: LGTM!The test file correctly imports from
@testing-library/preactandpreact/hooks, addressing the previous review feedback. Test coverage foruseIsFetchingis comprehensive, including real-time updates, render-phase safety, filtering, and custom queryClient usage.packages/preact-query/src/__tests__/HydrationBoundary.test.tsx (1)
34-483: Excellent hydration test coverage.The test suite comprehensively covers hydration scenarios including nested contexts, state changes, aborted transitions, invalid state handling, and the regression test for issue #8677 (infinite loop with error promises).
packages/preact-query/src/__tests__/useSuspenseQuery.test.tsx (1)
34-995: Comprehensive test coverage for useSuspenseQuery.The test suite thoroughly covers Suspense mode rendering, error boundaries, error reset behavior, staleTime functions, query switching, infinite queries, skipToken validation, and refetch intervals. The edge case tests for infinite loops and background errors are particularly valuable.
packages/preact-query/src/__tests__/usePrefetchInfiniteQuery.test.tsx (2)
16-41: LGTM! Helper function is well-structured.The
generateInfiniteQueryOptionshelper correctly creates mock paginated data with appropriate queryFn and pagination logic.
79-122: LGTM! Test cases are comprehensive.The test cases effectively validate:
- Prefetching behavior when query state is missing
- Cache population preventing fallback rendering
- Proper interaction with Suspense boundaries
The assertions check both UI state and queryFn invocation counts appropriately.
Also applies to: 124-160
packages/preact-query/src/__tests__/useQueries.test.tsx (5)
43-136: LGTM! Basic functionality tests are comprehensive.The tests effectively validate:
- Correct state transitions across multiple queries
- Timing behavior with different query durations
- Tracking and refetching with proper render count verification
138-697: LGTM! Type inference tests are thorough.Extensive type-level testing covering:
- Tuple-based type parameters (both tuple-of-tuples and tuple-of-objects)
- Type inference from array literals and Array.map()
- TData precedence over TQueryFnData
- Error type inference from throwOnError
- queryOptions type preservation
- Field name enforcement
The use of
@ts-expect-errorandexpectTypeOfappropriately validates TypeScript behavior.
804-936: LGTM! Error handling tests are well-designed.The tests properly validate throwOnError behavior:
- Boolean throwOnError correctly throws errors
- Function-based throwOnError respects return value
- Only the first failing query throws (correct short-circuit behavior)
- Console mocking prevents noise in test output
966-1743: LGTM! Combine function tests are exceptionally comprehensive.The test suite thoroughly validates combine behavior:
- Basic combine functionality with multiple queries
- Optimization with stable references (prevents unnecessary re-renders)
- Property tracking across observers
- Prevention of stale closures and infinite loops
- Proper re-execution when functional reference changes
- Structural sharing optimization
The tests reference specific GitHub issues (#6645, #6648, #7000) for excellent traceability and regression prevention.
1745-1814: LGTM! Edge case test prevents important regression.The test validates that dynamically removing queries doesn't cause infinite re-renders, checking both removal of the last query and removal of the first query. The render count assertion (< 10) provides a reasonable guard against performance regressions.
Overall, this test file is exceptionally well-structured with comprehensive coverage of runtime behavior, type inference, error handling, and performance characteristics.
| function renderWithSuspense(client: QueryClient, ui: React.ReactNode) { | ||
| return render( | ||
| <QueryClientProvider client={client}> | ||
| <Suspense fallback="loading">{ui}</Suspense> | ||
| </QueryClientProvider>, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Use Preact types instead of React types.
Line 8 uses React.ReactNode but this is a Preact package.
Apply this diff:
-function renderWithSuspense(client: QueryClient, ui: React.ReactNode) {
+function renderWithSuspense(client: QueryClient, ui: ComponentChildren) {
return render(
<QueryClientProvider client={client}>
<Suspense fallback="loading">{ui}</Suspense>
</QueryClientProvider>,
)
}Add the import if not already present:
import { QueryClient, QueryClientProvider, useSuspenseQuery } from '..'
import type { QueryKey } from '..'
import { Suspense } from 'preact/compat'
+import type { ComponentChildren } from 'preact'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function renderWithSuspense(client: QueryClient, ui: React.ReactNode) { | |
| return render( | |
| <QueryClientProvider client={client}> | |
| <Suspense fallback="loading">{ui}</Suspense> | |
| </QueryClientProvider>, | |
| ) | |
| } | |
| import { QueryClient, QueryClientProvider, useSuspenseQuery } from '..' | |
| import type { QueryKey } from '..' | |
| import { Suspense } from 'preact/compat' | |
| import type { ComponentChildren } from 'preact' | |
| function renderWithSuspense(client: QueryClient, ui: ComponentChildren) { | |
| return render( | |
| <QueryClientProvider client={client}> | |
| <Suspense fallback="loading">{ui}</Suspense> | |
| </QueryClientProvider>, | |
| ) | |
| } |
🤖 Prompt for AI Agents
In packages/preact-query/src/__tests__/suspense.test.tsx around lines 8 to 14,
the function uses the React type React.ReactNode but this is a Preact package;
import the appropriate Preact type (e.g., add "import { ComponentChildren } from
'preact'") if missing, and change the function signature to use
ComponentChildren (or another suitable Preact JSX type) instead of
React.ReactNode so the test uses Preact types consistently.
| it('should not create an endless loop when using inside a suspense boundary', async () => { | ||
| const queryOpts = { | ||
| queryKey: queryKey(), | ||
| ...generateInfiniteQueryOptions([ | ||
| { data: 'Infinite Page 1', currentPage: 1, totalPages: 3 }, | ||
| { data: 'Infinite Page 2', currentPage: 1, totalPages: 3 }, | ||
| { data: 'Infinite Page 3', currentPage: 1, totalPages: 3 }, | ||
| ]), |
There was a problem hiding this comment.
Fix incorrect currentPage values in test data.
Lines 167-168 have currentPage: 1 for pages 2 and 3, which is inconsistent with the test data structure and will break the getNextPageParam logic that relies on comparing currentPage with totalPages.
Apply this diff:
...generateInfiniteQueryOptions([
{ data: 'Infinite Page 1', currentPage: 1, totalPages: 3 },
- { data: 'Infinite Page 2', currentPage: 1, totalPages: 3 },
- { data: 'Infinite Page 3', currentPage: 1, totalPages: 3 },
+ { data: 'Infinite Page 2', currentPage: 2, totalPages: 3 },
+ { data: 'Infinite Page 3', currentPage: 3, totalPages: 3 },
]),🤖 Prompt for AI Agents
In packages/preact-query/src/__tests__/usePrefetchInfiniteQuery.test.tsx around
lines 162-169, the test data has incorrect currentPage values for pages 2 and 3
(both set to 1); update the second item to currentPage: 2 and the third item to
currentPage: 3 so the generated infinite query pages have correct sequential
currentPage values and the getNextPageParam logic can correctly compare
currentPage to totalPages.
| import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' | ||
| import { ErrorBoundary } from './utils' | ||
| import { | ||
| createRenderStream, | ||
| useTrackRenders, | ||
| } from '@testing-library/react-render-stream' | ||
| import { queryKey } from '@tanstack/query-test-utils' | ||
| import { waitFor } from '@testing-library/preact' | ||
| import { | ||
| QueryClient, | ||
| QueryClientProvider, | ||
| QueryErrorResetBoundary, | ||
| keepPreviousData, | ||
| useInfiniteQuery, | ||
| useQuery, | ||
| } from '..' | ||
| import { QueryCache } from '../index' | ||
| import { Suspense } from 'preact/compat' | ||
|
|
There was a problem hiding this comment.
Fix missing imports for useState, Fragment, and use.
The test uses useState (line 388, 1145, 1219, 1328), Fragment (line 1809), and use() function (lines 45, 111, 165, etc.) but none are imported.
Apply this diff to fix the imports:
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
import { ErrorBoundary } from './utils'
import {
createRenderStream,
useTrackRenders,
} from '@testing-library/react-render-stream'
import { queryKey } from '@tanstack/query-test-utils'
import { waitFor } from '@testing-library/preact'
import {
QueryClient,
QueryClientProvider,
QueryErrorResetBoundary,
keepPreviousData,
useInfiniteQuery,
useQuery,
} from '..'
import { QueryCache } from '../index'
import { Suspense } from 'preact/compat'
+import { useState, Fragment } from 'preact/compat'
+import { use } from 'preact/compat'Note: Verify that Preact exports use for React.use() compatibility. If not available in the version being used, this may require a polyfill or different approach.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/preact-query/src/__tests__/useQuery.promise.test.tsx around lines 1
to 19, the test file uses useState, Fragment, and the experimental use()
function but does not import them; add the missing imports (import useState and
Fragment from 'preact' or appropriate Preact entry, and import use from
'preact/hooks' if your Preact version exposes it there) at the top of the file
so the referenced lines (e.g. uses at 45, 111, 165, 388, 1145, 1219, 1328, 1809)
resolve; if your Preact version does not export use(), replace usages with an
equivalent pattern or add a polyfill and update imports accordingly.
…eact) -- fixes a lot of tests, which expect react-related functionality
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
packages/preact-query/src/__tests__/suspense.test.tsx (1)
8-8: Use Preact types instead of React types.The parameter type
React.ReactNodeshould be replaced with Preact'sComponentChildrentype.packages/preact-query/src/__tests__/useQuery.promise.test.tsx (1)
1-18: Missing imports foruseStateandusewill cause test failures.The test file uses
useState(lines 388, 964, 1063, 1143, 1219, 1328) anduse()(lines 45, 111, 165, and many more) without importing them. These need to be added for the tests to compile.This issue was already flagged in a previous review. Please add the missing imports:
import { Suspense } from 'preact/compat' +import { useState } from 'preact/hooks'For
use, verify whether Preact exposes this React 19 API viapreact/compat. If not, an alternative approach or polyfill may be needed.
🧹 Nitpick comments (17)
packages/preact-query/src/__tests__/useInfiniteQuery.test.tsx (4)
1-19: Import ordering inconsistency.The
Suspenseimport on line 19 is placed after the type imports (lines 13-18). For consistency with standard import conventions, runtime imports should be grouped together before type imports.import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { fireEvent, render } from '@testing-library/preact' import { queryKey, sleep } from '@tanstack/query-test-utils' import { useCallback, useEffect, useRef, useState } from 'preact/hooks' +import { Suspense } from 'preact/compat' import { QueryCache, QueryClient, QueryClientProvider, keepPreviousData, useInfiniteQuery, } from '..' import { renderWithClient, setActTimeout } from './utils' import type { InfiniteData, QueryFunctionContext, UseInfiniteQueryResult, } from '..' import type { Mock } from 'vitest' -import { Suspense } from 'preact/compat'
54-62: Consider test isolation for shared QueryClient.The
queryCacheandqueryClientare shared across all tests at the module level. While each test uses a uniquequeryKey(), residual cache state could potentially affect subsequent tests. Consider clearing the cache inafterEachor creating fresh instances per test.+ afterEach(() => { + queryCache.clear() + vi.useRealTimers() + }) - afterEach(() => { - vi.useRealTimers() - })
1225-1300: Commented-out test code should be addressed.This large commented-out test block references
createRenderStreamwhich appears to be a React-specific testing utility not available in Preact. Consider either:
- Implementing an equivalent test using Preact-compatible utilities
- Removing the commented code and tracking the missing coverage in a TODO comment or issue
Leaving ~75 lines of commented-out code reduces maintainability.
Would you like me to help implement an equivalent test using Preact-compatible testing patterns, or open an issue to track this?
1783-1859: Another large commented-out test block.Similar to the previous commented block, this test uses React-specific utilities (
createRenderStream,useTrackRenders) and React 19'suse()hook. Consider the same approach: either implement with Preact equivalents or remove with appropriate tracking.packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx (2)
27-28: Same test isolation concern as useInfiniteQuery tests.The shared
queryCacheandqueryClientat module level could benefit from cache clearing inafterEachto ensure complete test isolation.
643-666: Console mock is unnecessary in this test.This test (
should render children) doesn't throw any errors or trigger error boundaries, so the console mock setup/restore is unnecessary overhead.it('should render children', () => { - const consoleMock = vi - .spyOn(console, 'error') - .mockImplementation(() => undefined) - function Page() { return ( <div> <span>page</span> </div> ) } const rendered = renderWithClient( queryClient, <QueryErrorResetBoundary> <Page /> </QueryErrorResetBoundary>, ) expect(rendered.queryByText('page')).not.toBeNull() - - consoleMock.mockRestore() })examples/preact/simple/src/index.tsx (2)
18-26: Consider adding type safety for the API response.While acceptable for a simple example, adding a type annotation would demonstrate best practices and prevent runtime errors if the API response structure changes.
🔎 Example with type safety
Add an interface before the component:
interface RepoData { full_name: string description: string subscribers_count: number stargazers_count: number forks_count: number }Then update the useQuery call:
- const { isPending, error, data, isFetching } = useQuery({ + const { isPending, error, data, isFetching } = useQuery<RepoData>({ queryKey: ['repoData'], queryFn: async () => { const response = await fetch( 'https://api.github.com/repos/TanStack/query', ) - return await response.json() + return (await response.json()) as RepoData }, })
44-44: Consider adding a null check for the mount element.While unlikely to be an issue in this controlled example,
document.getElementById('app')could theoretically returnnull, causing a runtime error.🔎 Defensive approach
-render(<App />, document.getElementById('app')) +const root = document.getElementById('app') +if (root) { + render(<App />, root) +}packages/preact-query/src/__tests__/useQueries.test.tsx (3)
36-41: Consider adding cache cleanup for better test isolation.The
queryCacheandqueryClientare shared across all tests but never cleared between tests. While unique query keys help mitigate pollution, stale data from previous tests could theoretically affect later tests.🔎 Suggested improvement
afterEach(() => { vi.useRealTimers() + queryCache.clear() })
430-435:Array(50).map()creates a sparse array and won't produce 50 elements.
Array(50)creates an array with 50 empty slots (holes), and.map()skips holes. This results in an empty array at runtime. While this is currently in a type-only test (component not rendered), the pattern is misleading.🔎 Suggested fix using Array.from()
// Array.map preserves TQueryFnData const result1 = useQueries({ - queries: Array(50).map((_, i) => ({ + queries: Array.from({ length: 50 }, (_, i) => ({ queryKey: ['key', i] as const, queryFn: () => i + 10, })), })Apply the same fix to other occurrences at lines 445, 461, 472, 569, 576, 679, and 791.
1211-1214: Non-deterministic assertion may indicate test flakiness.The assertion
expect([4, 5, 6]).toContain(results.length)accepts a range of values, suggesting the exact number of renders isn't predictable. With fake timers, behavior should be deterministic. Consider investigating why the render count varies and either:
- Pin down the exact expected value, or
- Add a comment explaining why the variance is expected.
packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx (2)
774-808: Environment variable mutation could leak on test failure.If an assertion fails before
process.env.NODE_ENV = envCopyexecutes (lines 807, 841), subsequent tests will run with the wrongNODE_ENV. Consider usingtry/finallyto ensure cleanup.Suggested pattern
it('should log an error when skipToken is used in development environment', () => { const envCopy = process.env.NODE_ENV process.env.NODE_ENV = 'development' const consoleErrorSpy = vi .spyOn(console, 'error') .mockImplementation(() => undefined) const key = queryKey() // ... test body ... - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'skipToken is not allowed for useSuspenseQueries', - ) - consoleErrorSpy.mockRestore() - process.env.NODE_ENV = envCopy + try { + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'skipToken is not allowed for useSuspenseQueries', + ) + } finally { + consoleErrorSpy.mockRestore() + process.env.NODE_ENV = envCopy + } })Apply the same pattern to the production environment test (lines 810-842).
Also applies to: 810-842
677-684: Redundant timer setup in nested describe block.The parent
describe('useSuspenseQueries 2')already configures fake timers inbeforeEach/afterEach(lines 267-273). The nestedbeforeAll/afterAllhere is redundant.Suggested fix
describe('gc (with fake timers)', () => { - beforeAll(() => { - vi.useFakeTimers() - }) - - afterAll(() => { - vi.useRealTimers() - }) - it('should gc when unmounted while fetching with low gcTime (#8159)', async () => {packages/preact-query/src/__tests__/ErrorBoundary/ErrorBoundaryContext.ts (1)
3-7: Consider strengthening type safety.The use of
anytypes reduces type safety. Consider:
error: any→error: unknown(orerror: Errorif always Error)(...args: any[])→(...args: unknown[])This applies to test utilities as well, helping catch type issues during development.
🔎 Proposed type improvements
export type ErrorBoundaryContextType = { didCatch: boolean - error: any + error: unknown - resetErrorBoundary: (...args: any[]) => void + resetErrorBoundary: (...args: unknown[]) => void }packages/preact-query/src/__tests__/ErrorBoundary/types.ts (2)
8-11: Consider strengthening type safety in FallbackProps.Similar to ErrorBoundaryContextType, these
anytypes reduce type safety:
error: any→error: unknown(...args: any[])→(...args: unknown[])🔎 Proposed type improvements
export type FallbackProps = { - error: any + error: unknown - resetErrorBoundary: (...args: any[]) => void + resetErrorBoundary: (...args: unknown[]) => void }
17-25: Consider strengthening type for resetKeys.The
resetKeys?: any[]type is overly permissive. ConsiderresetKeys?: unknown[]for better type safety while maintaining flexibility for comparison purposes.🔎 Proposed type improvement
type ErrorBoundarySharedProps = PropsWithChildren<{ onError?: (error: Error, info: ErrorInfo) => void onReset?: ( details: | { reason: 'imperative-api'; args: any[] } | { reason: 'keys'; prev: any[] | undefined; next: any[] | undefined }, ) => void - resetKeys?: any[] + resetKeys?: unknown[] }>packages/preact-query/src/__tests__/ErrorBoundary/ErrorBoundary.ts (1)
97-109: Runtime fallback validation is reasonable but could error earlier.The TypeScript types should prevent missing fallback props at compile time through the union types. However, this runtime check (lines 104-108) serves as a safety net. Consider whether validation in the constructor would provide earlier feedback, though the current approach is acceptable given the type safety already in place.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
examples/preact/simple/package.jsonexamples/preact/simple/src/index.tsxpackages/preact-query/package.jsonpackages/preact-query/src/QueryErrorResetBoundary.tsxpackages/preact-query/src/__tests__/ErrorBoundary/ErrorBoundary.tspackages/preact-query/src/__tests__/ErrorBoundary/ErrorBoundaryContext.tspackages/preact-query/src/__tests__/ErrorBoundary/index.tspackages/preact-query/src/__tests__/ErrorBoundary/types.tspackages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsxpackages/preact-query/src/__tests__/suspense.test.tsxpackages/preact-query/src/__tests__/useInfiniteQuery.test.tsxpackages/preact-query/src/__tests__/useMutation.test.tsxpackages/preact-query/src/__tests__/usePrefetchQuery.test.tsxpackages/preact-query/src/__tests__/useQueries.test.tsxpackages/preact-query/src/__tests__/useQuery.promise.test.tsxpackages/preact-query/src/__tests__/useQuery.test.tsxpackages/preact-query/src/__tests__/useSuspenseQueries.test.tsxpackages/preact-query/src/__tests__/useSuspenseQuery.test.tsxpackages/preact-query/src/__tests__/utils.tsxpackages/preact-query/src/useSuspenseQuery.ts
✅ Files skipped from review due to trivial changes (1)
- packages/preact-query/src/tests/ErrorBoundary/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/preact-query/src/tests/utils.tsx
- packages/preact-query/src/tests/usePrefetchQuery.test.tsx
- packages/preact-query/src/tests/useMutation.test.tsx
- examples/preact/simple/package.json
- packages/preact-query/src/useSuspenseQuery.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: TkDodo
Repo: TanStack/query PR: 9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.184Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
Learnt from: oscartbeaumont
Repo: TanStack/query PR: 9564
File: packages/solid-query-devtools/src/production.tsx:2-3
Timestamp: 2025-08-19T03:18:18.303Z
Learning: In the solid-query-devtools package, the codebase uses a pattern of type-only default imports combined with typeof for component type annotations (e.g., `import type SolidQueryDevtoolsComp from './devtools'` followed by `typeof SolidQueryDevtoolsComp`). This pattern is consistently used across index.tsx and production.tsx files, and the maintainers prefer consistency over changing this approach.
📚 Learning: 2025-11-22T09:06:05.219Z
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
Applied to files:
packages/preact-query/src/__tests__/suspense.test.tsxpackages/preact-query/src/__tests__/useQuery.promise.test.tsxpackages/preact-query/src/__tests__/useSuspenseQuery.test.tsxpackages/preact-query/src/__tests__/useQueries.test.tsxpackages/preact-query/src/__tests__/useInfiniteQuery.test.tsxpackages/preact-query/package.jsonpackages/preact-query/src/__tests__/useSuspenseQueries.test.tsxpackages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx
📚 Learning: 2025-08-19T03:18:18.303Z
Learnt from: oscartbeaumont
Repo: TanStack/query PR: 9564
File: packages/solid-query-devtools/src/production.tsx:2-3
Timestamp: 2025-08-19T03:18:18.303Z
Learning: In the solid-query-devtools package, the codebase uses a pattern of type-only default imports combined with typeof for component type annotations (e.g., `import type SolidQueryDevtoolsComp from './devtools'` followed by `typeof SolidQueryDevtoolsComp`). This pattern is consistently used across index.tsx and production.tsx files, and the maintainers prefer consistency over changing this approach.
Applied to files:
packages/preact-query/src/__tests__/suspense.test.tsxpackages/preact-query/src/__tests__/useQuery.promise.test.tsxpackages/preact-query/src/__tests__/ErrorBoundary/types.tspackages/preact-query/src/__tests__/useQueries.test.tsxpackages/preact-query/package.json
📚 Learning: 2025-09-02T17:57:33.184Z
Learnt from: TkDodo
Repo: TanStack/query PR: 9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.184Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
Applied to files:
packages/preact-query/src/__tests__/useQuery.promise.test.tsxpackages/preact-query/src/__tests__/useQueries.test.tsxpackages/preact-query/package.json
📚 Learning: 2025-11-02T22:52:33.071Z
Learnt from: DogPawHat
Repo: TanStack/query PR: 9835
File: packages/query-core/src/__tests__/queryClient.test-d.tsx:242-256
Timestamp: 2025-11-02T22:52:33.071Z
Learning: In the TanStack Query codebase, the new `query` and `infiniteQuery` methods support the `select` option for data transformation, while the legacy `fetchQuery` and `fetchInfiniteQuery` methods do not support `select` and should reject it at the type level.
Applied to files:
packages/preact-query/src/__tests__/useQueries.test.tsxpackages/preact-query/src/__tests__/useInfiniteQuery.test.tsx
🧬 Code graph analysis (6)
packages/preact-query/src/__tests__/useQuery.promise.test.tsx (6)
packages/preact-query/src/index.ts (4)
useQuery(10-10)QueryClientProvider(34-34)QueryErrorResetBoundary(48-48)useInfiniteQuery(55-55)packages/preact-query/src/useQuery.ts (1)
useQuery(50-52)packages/preact-query/src/QueryClientProvider.tsx (1)
QueryClientProvider(30-46)packages/preact-query/src/QueryErrorResetBoundary.tsx (1)
QueryErrorResetBoundary(47-56)packages/preact-query/src/__tests__/ErrorBoundary/ErrorBoundary.ts (2)
ErrorBoundary(20-124)resetErrorBoundary(35-46)packages/preact-query/src/useInfiniteQuery.ts (1)
useInfiniteQuery(72-81)
packages/preact-query/src/__tests__/useSuspenseQuery.test.tsx (5)
packages/preact-query/src/types.ts (2)
UseSuspenseQueryResult(160-166)UseSuspenseInfiniteQueryResult(183-189)packages/preact-query/src/useSuspenseQuery.ts (1)
useSuspenseQuery(7-33)packages/preact-query/src/__tests__/utils.tsx (1)
renderWithClient(10-24)packages/preact-query/src/useSuspenseInfiniteQuery.ts (1)
useSuspenseInfiniteQuery(18-50)packages/preact-query/src/QueryErrorResetBoundary.tsx (2)
QueryErrorResetBoundary(47-56)useQueryErrorResetBoundary(34-35)
packages/preact-query/src/__tests__/useQueries.test.tsx (4)
packages/preact-query/src/types.ts (2)
UseQueryResult(155-158)UseQueryOptions(65-73)packages/preact-query/src/index.ts (2)
useQueries(8-8)queryOptions(20-20)packages/preact-query/src/__tests__/utils.tsx (1)
renderWithClient(10-24)packages/preact-query/src/queryOptions.ts (1)
queryOptions(85-87)
packages/preact-query/src/QueryErrorResetBoundary.tsx (1)
packages/preact-query/src/index.ts (7)
QueryErrorResetFunction(45-45)QueryErrorIsResetFunction(43-43)QueryErrorClearResetFunction(42-42)useQueryErrorResetBoundary(49-49)QueryErrorResetBoundaryFunction(44-44)QueryErrorResetBoundaryProps(38-38)QueryErrorResetBoundary(48-48)
packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx (3)
packages/preact-query/src/types.ts (1)
UseSuspenseQueryOptions(81-94)packages/preact-query/src/useSuspenseQueries.ts (1)
useSuspenseQueries(189-211)packages/preact-query/src/useSuspenseQuery.ts (1)
useSuspenseQuery(7-33)
packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx (8)
packages/preact-query/src/index.ts (5)
useQuery(10-10)QueryErrorResetBoundary(48-48)useSuspenseQuery(11-11)useQueries(8-8)useSuspenseQueries(13-13)packages/preact-query/src/useQuery.ts (1)
useQuery(50-52)packages/preact-query/src/__tests__/utils.tsx (1)
renderWithClient(10-24)packages/preact-query/src/QueryErrorResetBoundary.tsx (1)
QueryErrorResetBoundary(47-56)packages/preact-query/src/__tests__/ErrorBoundary/ErrorBoundary.ts (2)
ErrorBoundary(20-124)resetErrorBoundary(35-46)packages/preact-query/src/useSuspenseQuery.ts (1)
useSuspenseQuery(7-33)packages/preact-query/src/useQueries.ts (1)
useQueries(212-335)packages/preact-query/src/useSuspenseQueries.ts (1)
useSuspenseQueries(189-211)
🔇 Additional comments (21)
packages/preact-query/src/__tests__/useInfiniteQuery.test.tsx (1)
64-156: LGTM - comprehensive state assertions.This test thoroughly validates the initial and success states of
useInfiniteQuery, including all expected properties like pagination flags, fetch status, and data structure. Good coverage of the hook's state machine.packages/preact-query/src/__tests__/QueryResetErrorBoundary.test.tsx (2)
31-90: LGTM - solid error boundary reset test.This test properly validates the core use case: retrying a failed query after resetting the error boundary. The test structure with
succeedflag, console mock, and proper timer advancement is well-implemented.
731-866: Good coverage of useQueries with error boundaries.The
useQueriesdescribe block appropriately tests both the standard and suspense variants (useSuspenseQueries) with error boundary integration. TheretryOnMount: trueoption is correctly used to ensure retry behavior on remount.packages/preact-query/src/QueryErrorResetBoundary.tsx (1)
1-56: LGTM - clean implementation of QueryErrorResetBoundary.The implementation correctly follows the established pattern from React Query:
- Uses closure-based state management in
createValue()for the reset flag- Provides both a hook (
useQueryErrorResetBoundary) and component API- Supports function-as-children pattern for accessing the boundary value
- Each
QueryErrorResetBoundaryinstance gets its own isolated state viauseState(() => createValue())The type exports align with what's declared in
index.ts.examples/preact/simple/src/index.tsx (1)
1-6: LGTM!The imports are clean and correctly reference the new
@tanstack/preact-querypackage.packages/preact-query/src/__tests__/useQueries.test.tsx (2)
790-800: Verify the expected type forwithWrappedQueries.The test uses
getSelectorA()which returns[number, string], but the type expectation isArray<UseQueryResult<number, Error>>. This suggests theselecttransformation is lost through the wrapper. If this is intentional to demonstrate the limitation of the generic wrapper, consider adding a comment. Otherwise, the expected type should beArray<UseQueryResult<[number, string], Error>>.
31-88: Comprehensive test coverage for useQueries hook.The test suite thoroughly covers:
- Basic state tracking and timing behavior
- Type inference for tuples, objects, and arrays
- Error boundary integration with
throwOnError- Combine function behavior and optimization
- Re-render prevention and stability
- Regression tests for specific GitHub issues (#6645, #6648, #7000)
The use of fake timers and unique query keys per test provides good isolation.
packages/preact-query/src/__tests__/ErrorBoundary/ErrorBoundary.ts (5)
20-29: LGTM!Constructor properly binds the reset method and initializes state. Standard React/Preact pattern.
31-33: LGTM!Correct implementation of the static error lifecycle method.
48-56: LGTM!The intentional
console.erroris well-documented and emulates React's behavior for error visibility. The implementation correctly invokes the optionalonErrorcallback.
58-83: LGTM!Excellent edge case handling with clear documentation. The check for
prevState.error !== nullprevents premature resets when resetKeys change simultaneously with an error being thrown.
126-130: LGTM!The
hasArrayChangedhelper correctly usesObject.isfor value comparison and handles length differences. The implementation is sound and efficient.packages/preact-query/src/__tests__/ErrorBoundary/types.ts (1)
13-15: No action needed. Preact v10 does not export aPropsWithChildrentype. The custom utility type defined here is appropriate and necessary for combining generic props with optional children in a reusable way.Likely an incorrect or invalid review comment.
packages/preact-query/src/__tests__/useQuery.promise.test.tsx (5)
20-37: Test setup looks correct.The shared
QueryClientandQueryCachepattern is acceptable here since each test uses a uniquequeryKey()for isolation. ThebeforeAll/afterAllhooks properly manage fake timers and the experimental option.
39-92: Well-structured test with good render tracking.The test properly verifies the suspense flow by checking rendered components and DOM state at each render phase. The pattern of using
createRenderStreamwithtakeRender()provides clear visibility into the component lifecycle.
494-584: Comprehensive error boundary integration test.Good coverage of the error → reset → retry flow with proper console mock management. The test verifies both the error state display and successful recovery after
resetErrorBoundary().
1305-1383: Good test for observer lifecycle management.The assertions on
observers.length(lines 1357-1382) properly verify that observers are attached when components mount and detached when queryKey changes. This is important for preventing memory leaks.
1385-1430: Interesting test for implicit observation.This test verifies that calling
use(query.promise)implicitly subscribes to query updates, allowingquery.hasNextPageto reflect the correct state. Good coverage of theuseInfiniteQueryintegration with the promise API.packages/preact-query/package.json (3)
67-69: LGTM! Workspace dependency correctly configured.The
@tanstack/query-coreworkspace dependency follows the established pattern for TanStack Query packages.Based on learnings, this is the correct way to reference query-core from other packages in the monorepo.
3-3: No action required. The version5.90.11correctly aligns with the monorepo's unified versioning strategy, matching @tanstack/query-core (5.90.11) and other framework adapters like @tanstack/react-query (5.90.11).
84-86: The peer dependency range^10.0.0is appropriate and follows official Preact best practices. Preact 10.x maintains full backward compatibility from 10.0.0 through 10.28.0 with no breaking changes between versions. The caret constraint correctly allows all 10.x releases while preventing accidental installation of the breaking 11.x. The package already follows recommended practice by testing against^10.28.0in devDependencies.
| export function App() { | ||
| const queryClient = new QueryClient() | ||
| return ( | ||
| <QueryClientProvider client={queryClient}> | ||
| <Example /> | ||
| </QueryClientProvider> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Move QueryClient instantiation outside the component.
Creating a new QueryClient inside the component body causes it to be recreated on every render, which defeats the purpose of query caching and can lead to memory leaks. The QueryClient should be instantiated once at the module level or memoized.
🔎 Proposed fix
+const queryClient = new QueryClient()
+
export function App() {
- const queryClient = new QueryClient()
return (
<QueryClientProvider client={queryClient}>
<Example />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function App() { | |
| const queryClient = new QueryClient() | |
| return ( | |
| <QueryClientProvider client={queryClient}> | |
| <Example /> | |
| </QueryClientProvider> | |
| ) | |
| } | |
| const queryClient = new QueryClient() | |
| export function App() { | |
| return ( | |
| <QueryClientProvider client={queryClient}> | |
| <Example /> | |
| </QueryClientProvider> | |
| ) | |
| } |
🤖 Prompt for AI Agents
In examples/preact/simple/src/index.tsx around lines 8 to 15, the QueryClient is
instantiated inside the App component causing a new client on every render; move
the QueryClient creation to module scope (instantiate it once at top-level) or
memoize it with a stable initializer so the same client is reused across
renders, then pass that single instance into QueryClientProvider.
| queryFn: async () => { | ||
| const response = await fetch( | ||
| 'https://api.github.com/repos/TanStack/query', | ||
| ) | ||
| return await response.json() | ||
| }, |
There was a problem hiding this comment.
Add response status validation.
The fetch call doesn't check response.ok, so non-2xx HTTP responses will still attempt JSON parsing and may not be caught properly as errors. This could lead to confusing error messages or unhandled failures.
🔎 Proposed fix
queryFn: async () => {
const response = await fetch(
'https://api.github.com/repos/TanStack/query',
)
+ if (!response.ok) {
+ throw new Error(`HTTP error! status: ${response.status}`)
+ }
return await response.json()
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| queryFn: async () => { | |
| const response = await fetch( | |
| 'https://api.github.com/repos/TanStack/query', | |
| ) | |
| return await response.json() | |
| }, | |
| queryFn: async () => { | |
| const response = await fetch( | |
| 'https://api.github.com/repos/TanStack/query', | |
| ) | |
| if (!response.ok) { | |
| throw new Error(`HTTP error! status: ${response.status}`) | |
| } | |
| return await response.json() | |
| }, |
🤖 Prompt for AI Agents
In examples/preact/simple/src/index.tsx around lines 20 to 25, the fetch
response is parsed without validating HTTP status; update the queryFn to check
response.ok before calling response.json(), and if not ok throw an error
containing the response status and statusText (optionally including the response
body text/json) so non-2xx responses are surfaced as errors to the caller.
| import { | ||
| createRenderStream, | ||
| useTrackRenders, | ||
| } from '@testing-library/react-render-stream' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find config files in the packages/preact-query directory
fd -type f -name "vite.config.*" -o -name "vitest.config.*" -o -name "tsconfig.json" -o -name "package.json" packages/preact-query | head -20Repository: TanStack/query
Length of output: 228
🏁 Script executed:
# Check for alias configuration in the root and packages level
rg -l "alias|react.*preact" packages/preact-query --type json --type ts --type jsRepository: TanStack/query
Length of output: 192
🏁 Script executed:
# Look at the test file itself for more context
cat -n packages/preact-query/src/__tests__/useQuery.promise.test.tsx | head -30Repository: TanStack/query
Length of output: 1112
🏁 Script executed:
cat packages/preact-query/vite.config.tsRepository: TanStack/query
Length of output: 825
🏁 Script executed:
# Also check package.json for test setup and dependencies
cat packages/preact-query/package.jsonRepository: TanStack/query
Length of output: 3313
🏁 Script executed:
# Check root vite or vitest config
find . -maxdepth 2 -name "vite.config.*" -o -name "vitest.config.*" | grep -v node_modules | grep -v packagesRepository: TanStack/query
Length of output: 40
🏁 Script executed:
# Check if there's pnpm config or setup that might handle aliasing
cat pnpm-workspace.yaml 2>/dev/null || echo "No workspace config found"Repository: TanStack/query
Length of output: 357
🏁 Script executed:
# Look for any alias configuration across the repo
rg "alias.*react|react.*alias" --type json --type ts -g "!node_modules"Repository: TanStack/query
Length of output: 40
🌐 Web query:
@testing-library/react-render-stream Preact compatibility
💡 Result:
Short answer: No — @testing-library/react-render-stream is React-specific and not compatible with Preact. It depends on React runtime behavior (uses React's render/act/IS_REACT_ACT_ENVIRONMENT semantics), whereas Preact has its own testing package and different event/act behavior; use @testing-library/preact (or Preact-specific testing utilities) instead. [1][2]
Sources:
- @testing-library/react-render-stream README (React-focused, mentions act / IS_REACT_ACT_ENVIRONMENT). [1]
- Testing Library — Preact docs / @testing-library/preact guidance. [2]
🏁 Script executed:
# Check if createRenderStream and useTrackRenders are actually used in the test
grep -n "createRenderStream\|useTrackRenders" packages/preact-query/src/__tests__/useQuery.promise.test.tsxRepository: TanStack/query
Length of output: 2531
Replace @testing-library/react-render-stream with Preact-compatible alternatives.
@testing-library/react-render-stream is React-specific and cannot work with Preact—even with module aliasing. The library depends on React's runtime semantics (act behavior, render lifecycle) which differ fundamentally from Preact's. These utilities are used extensively throughout the test file (40+ usages across all test cases) and will cause runtime failures. Use Preact-compatible testing utilities instead.
🤖 Prompt for AI Agents
In packages/preact-query/src/__tests__/useQuery.promise.test.tsx around lines 3
to 6, the test currently imports React-only utilities from
@testing-library/react-render-stream which are incompatible with Preact; replace
that import with Preact-compatible testing utilities (for example, swap to
@testing-library/preact for DOM helpers and use preact/test-utils (act) and
preact-render-to-string or a Preact-compatible stream renderer) and then update
all usages (createRenderStream, useTrackRenders, etc.) to the equivalent
Preact-based functions: import the renderer and act from Preact libs, convert
stream-based assertions to the Preact renderer’s API (or render-to-string
snapshots) and adjust async/act wrappers to use preact/test-utils so the ~40
usages in this file call Preact-compatible functions instead of the React-only
ones.
| it('should not suspend on mount if query has been already fetched', () => { | ||
| const query = createQuery(1) | ||
|
|
||
| queryClient.setQueryData(query.queryKey, query.queryFn) | ||
|
|
||
| render(<TestComponent queries={[query]} />) | ||
|
|
||
| expect(onSuspend).not.toHaveBeenCalled() | ||
| }) |
There was a problem hiding this comment.
Incorrect data passed to setQueryData.
setQueryData expects the resolved data value, but query.queryFn is the function itself, not its result. The cache is set to a function reference rather than the expected number 1.
Suggested fix
it('should not suspend on mount if query has been already fetched', () => {
const query = createQuery(1)
- queryClient.setQueryData(query.queryKey, query.queryFn)
+ queryClient.setQueryData(query.queryKey, 1)
render(<TestComponent queries={[query]} />)
expect(onSuspend).not.toHaveBeenCalled()
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should not suspend on mount if query has been already fetched', () => { | |
| const query = createQuery(1) | |
| queryClient.setQueryData(query.queryKey, query.queryFn) | |
| render(<TestComponent queries={[query]} />) | |
| expect(onSuspend).not.toHaveBeenCalled() | |
| }) | |
| it('should not suspend on mount if query has been already fetched', () => { | |
| const query = createQuery(1) | |
| queryClient.setQueryData(query.queryKey, 1) | |
| render(<TestComponent queries={[query]} />) | |
| expect(onSuspend).not.toHaveBeenCalled() | |
| }) |
🤖 Prompt for AI Agents
In packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx around lines
114 to 122, setQueryData is being called with query.queryFn (the function)
instead of the resolved value; change the call to pass the actual resolved data
(e.g., the number 1 or await the function) so the cache contains the expected
data value rather than a function reference.
| describe('useSuspenseQueries 2', () => { | ||
| beforeEach(() => { | ||
| vi.useFakeTimers() | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| vi.useRealTimers() | ||
| }) |
There was a problem hiding this comment.
Missing queryClient.clear() in second describe block's afterEach.
The second describe block reuses the module-level queryClient (e.g., lines 315, 372, 418, 468, etc.) but only resets timers in afterEach. This can cause test pollution if cached query data from one test affects subsequent tests.
Suggested fix
afterEach(() => {
vi.useRealTimers()
+ queryClient.clear()
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe('useSuspenseQueries 2', () => { | |
| beforeEach(() => { | |
| vi.useFakeTimers() | |
| }) | |
| afterEach(() => { | |
| vi.useRealTimers() | |
| }) | |
| describe('useSuspenseQueries 2', () => { | |
| beforeEach(() => { | |
| vi.useFakeTimers() | |
| }) | |
| afterEach(() => { | |
| vi.useRealTimers() | |
| queryClient.clear() | |
| }) |
🤖 Prompt for AI Agents
In packages/preact-query/src/__tests__/useSuspenseQueries.test.tsx around lines
266 to 273, the second describe block resets fake timers in afterEach but does
not clear the shared module-level queryClient, risking cross-test cache
pollution; update the afterEach to call queryClient.clear() (or await
queryClient.clear() if async) in addition to vi.useRealTimers() so each test
starts with a fresh cache.
3 participants
🎯 Changes
Makes a Preact Adapter for Tanstack Query. The main motivations are bundle sizes and ensuring compatibility with the events architecture.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.