Feat: Preact Adapter #9935
Feat: Preact Adapter #9935
Conversation
|
WalkthroughAdds a new Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas to focus review on:
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)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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
| "dependencies": { | ||
| "preact": "^10.26.9" | ||
| }, |
There was a problem hiding this comment.
Missing @tanstack/preact-query dependency.
This example is meant to demonstrate the Preact Query adapter, but the package doesn't include @tanstack/preact-query in its dependencies. Based on similar examples in the TanStack Query monorepo, workspace dependencies should be added.
"dependencies": {
- "preact": "^10.26.9"
+ "preact": "^10.26.9",
+ "@tanstack/preact-query": "workspace:*"
},📝 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.
| "dependencies": { | |
| "preact": "^10.26.9" | |
| }, | |
| "dependencies": { | |
| "preact": "^10.26.9", | |
| "@tanstack/preact-query": "workspace:*" | |
| }, |
🤖 Prompt for AI Agents
In examples/preact/simple/package.json around lines 9 to 11, the package.json is
missing the @tanstack/preact-query dependency required for the Preact Query
adapter example; add @tanstack/preact-query (use a workspace or matching version
as other examples in the monorepo use) to the "dependencies" section so the
example installs and runs correctly, ensuring version aligns with the monorepo's
TanStack Query packages.
| // @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.
| '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.
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.