Conversation
|
github-actions
bot
added
the
documentation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughA new Preact query persist client package is introduced with full configuration, including TypeScript and ESLint setup, a PersistQueryClientProvider component for managing persistent state, documentation pages for Preact plugins, and comprehensive tests covering restoration and memoized query operations. Changes
Sequence DiagramsequenceDiagram
participant Component as Page Component
participant Provider as PersistQueryClientProvider
participant Core as persistQueryClientCore
participant Client as QueryClient
participant Storage as Persister/Storage
Component->>Provider: Render with children
Provider->>Provider: Initialize state (isRestoring=true)
Provider->>Core: persistQueryClientRestore(options)
Core->>Storage: Restore persisted state
Storage-->>Core: Return cached state
Core-->>Client: Hydrate QueryClient
Core-->>Provider: onSuccess/onError callback
Provider->>Provider: Set isRestoring=false
Provider->>Core: persistQueryClientSubscribe(options)
Core-->>Provider: Listen for future updates
Provider->>Component: Render QueryClientProvider + IsRestoringProvider
Component->>Client: useQuery() fetches data
Client-->>Component: Return query state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
🤖 Fix all issues with AI agents
In `@packages/preact-query-persist-client/src/PersistQueryClientProvider.tsx`:
- Around line 1-2: The import order is incorrect: move the type import "import
type { VNode } from 'preact'" so it appears after the package imports from
"@tanstack/*" (specifically after any import from "@tanstack/preact-query") to
satisfy ESLint ordering; update the top of PersistQueryClientProvider.tsx so
non-type `@tanstack` imports come first and the "VNode" type import follows them.
- Around line 33-48: The finally handler calls setIsRestoring(false) even after
unmount, causing state updates on unmounted component; inside the useEffect that
references didRestore, refs.current, persistQueryClientRestore, isRestoring and
persistQueryClientSubscribe, add a mounted flag (e.g., let mounted = true) and
set mounted = false in the cleanup, then only call setIsRestoring(false) in the
persistQueryClientRestore promise chain if mounted is true; ensure the cleanup
still returns persistQueryClientSubscribe(options) when not restoring and the
mounted flag prevents setIsRestoring from running after unmount.
In `@packages/preact-query-persist-client/vite.config.ts`:
- Around line 3-5: Reorder the imports so the package JSON import appears before
the Vite type import to satisfy import/order; move "import packageJson from
'./package.json'" above "import type { UserConfig as ViteUserConfig } from
'vite'"; ensure symbols packageJson and ViteUserConfig remain unchanged and that
no other code is affected by the swap.
🧹 Nitpick comments (1)
packages/preact-query-persist-client/src/PersistQueryClientProvider.tsx (1)
14-15:onErrorcallback doesn't receive the error object.The
onErrorsignature is() => ..., so the consumer has no way to inspect or log the actual error. Consider passing the caught error through:Suggested changes:
- onError?: () => Promise<unknown> | unknown + onError?: (err: unknown) => Promise<unknown> | unknownand on line 42:
- .catch(() => refs.current.onError?.()) + .catch((err) => refs.current.onError?.(err))Note: The React counterpart has the identical issue with the same signature and error handling pattern.
| import { useEffect, useRef, useState } from 'preact/hooks' | ||
| import type { VNode } from 'preact' |
There was a problem hiding this comment.
Fix import order: type import from preact should follow @tanstack/* imports.
ESLint reports that the preact type import on Line 2 should occur after the @tanstack/preact-query import.
🔧 Proposed fix
import { useEffect, useRef, useState } from 'preact/hooks'
-import type { VNode } from 'preact'
import {
persistQueryClientRestore,
persistQueryClientSubscribe,
} from '@tanstack/query-persist-client-core'
import { IsRestoringProvider, QueryClientProvider } from '@tanstack/preact-query'
import type { PersistQueryClientOptions } from '@tanstack/query-persist-client-core'
import type { OmitKeyof, QueryClientProviderProps } from '@tanstack/preact-query'
+import type { VNode } 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.
| import { useEffect, useRef, useState } from 'preact/hooks' | |
| import type { VNode } from 'preact' | |
| import { useEffect, useRef, useState } from 'preact/hooks' | |
| import { | |
| persistQueryClientRestore, | |
| persistQueryClientSubscribe, | |
| } from '@tanstack/query-persist-client-core' | |
| import { IsRestoringProvider, QueryClientProvider } from '@tanstack/preact-query' | |
| import type { PersistQueryClientOptions } from '@tanstack/query-persist-client-core' | |
| import type { OmitKeyof, QueryClientProviderProps } from '@tanstack/preact-query' | |
| import type { VNode } from 'preact' |
🧰 Tools
🪛 ESLint
[error] 2-2: preact type import should occur after import of @tanstack/preact-query
(import/order)
🤖 Prompt for AI Agents
In `@packages/preact-query-persist-client/src/PersistQueryClientProvider.tsx`
around lines 1 - 2, The import order is incorrect: move the type import "import
type { VNode } from 'preact'" so it appears after the package imports from
"@tanstack/*" (specifically after any import from "@tanstack/preact-query") to
satisfy ESLint ordering; update the top of PersistQueryClientProvider.tsx so
non-type `@tanstack` imports come first and the "VNode" type import follows them.
| useEffect(() => { | ||
| const options = { | ||
| ...refs.current.persistOptions, | ||
| queryClient: props.client, | ||
| } | ||
| if (!didRestore.current) { | ||
| didRestore.current = true | ||
| persistQueryClientRestore(options) | ||
| .then(() => refs.current.onSuccess?.()) | ||
| .catch(() => refs.current.onError?.()) | ||
| .finally(() => { | ||
| setIsRestoring(false) | ||
| }) | ||
| } | ||
| return isRestoring ? undefined : persistQueryClientSubscribe(options) | ||
| }, [props.client, isRestoring]) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd . && find . -type f -name "*PersistQueryClientProvider*" | head -20Repository: TanStack/query
Length of output: 617
🏁 Script executed:
ls -la packages/Repository: TanStack/query
Length of output: 1931
🏁 Script executed:
find . -name "PersistQueryClientProvider.tsx" -type fRepository: TanStack/query
Length of output: 277
🏁 Script executed:
git ls-files | grep "PersistQueryClientProvider"Repository: TanStack/query
Length of output: 603
🏁 Script executed:
git ls-files "**/*PersistQueryClientProvider*"Repository: TanStack/query
Length of output: 603
🏁 Script executed:
find packages -type f -name "*.tsx" | grep -i providerRepository: TanStack/query
Length of output: 855
🏁 Script executed:
find . -type f -name "*.tsx" -o -name "*.ts" | xargs grep -l "PersistQueryClientProvider" 2>/dev/nullRepository: TanStack/query
Length of output: 953
🏁 Script executed:
tree -L 3 -d 2>/dev/null || find . -maxdepth 3 -type d | head -30Repository: TanStack/query
Length of output: 6546
🏁 Script executed:
pwd && ls -laRepository: TanStack/query
Length of output: 2437
🏁 Script executed:
find . -path "*/preact-query-persist-client/*" -type fRepository: TanStack/query
Length of output: 623
🏁 Script executed:
rg "didRestore.current" -t tsx -t tsRepository: TanStack/query
Length of output: 85
🏁 Script executed:
git ls-files | wc -l && git ls-files | grep -i providerRepository: TanStack/query
Length of output: 2578
🏁 Script executed:
git ls-files | grep "\.tsx$"Repository: TanStack/query
Length of output: 17194
🏁 Script executed:
git log --oneline --all | head -5Repository: TanStack/query
Length of output: 340
🏁 Script executed:
git ls-files --full-name | sortRepository: TanStack/query
Length of output: 50371
🏁 Script executed:
git ls-files --full-name | grep -i persistRepository: TanStack/query
Length of output: 9283
🏁 Script executed:
find . -type f -name "*.tsx" | wc -lRepository: TanStack/query
Length of output: 61
🏁 Script executed:
rg "useEffect" --type tsx --type ts | grep -i "persistquery\|restore" -A 5Repository: TanStack/query
Length of output: 85
🏁 Script executed:
rg "persistQueryClientRestore"Repository: TanStack/query
Length of output: 2830
🏁 Script executed:
git ls-files | head -100Repository: TanStack/query
Length of output: 4804
🏁 Script executed:
cat /dev/null && pwdRepository: TanStack/query
Length of output: 76
🏁 Script executed:
git remote -v 2>/dev/null || echo "No git remote found"Repository: TanStack/query
Length of output: 174
🏁 Script executed:
git rev-parse --show-toplevel 2>/dev/null || echo "Not a git repo"Repository: TanStack/query
Length of output: 76
🏁 Script executed:
test -f packages/preact-query-persist-client/src/PersistQueryClientProvider.tsx && echo "File exists" || echo "File not found"Repository: TanStack/query
Length of output: 69
🏁 Script executed:
find . -iname "*react*query*persist*" -o -iname "*preact*query*persist*" 2>/dev/nullRepository: TanStack/query
Length of output: 134
🌐 Web query:
TanStack query persist client PersistQueryClientProvider restore unmount
💡 Result:
In TanStack Query (v5), restoration/persistence works like this:
persistQueryClient({ ... })does two things: (1) async restore of the persisted cache, then (2) subscribes to cache changes and returns an unsubscribe function. If you call it outside React, you can’t reliably unsubscribe and you can also get restore-vs-fetch race conditions. [1]PersistQueryClientProviderexists specifically to solve those lifecycle issues: it subscribes/unsubscribes with the React component lifecycle, and it also prevents queries from starting network fetching while restore is in progress (queries render, but are held infetchingState: 'idle'until restore completes). [1]onSuccessis called when the initial restore finishes; if it returns a Promise, the provider will keep “restoring” true until that Promise resolves (useful forresumePausedMutations). [1]
What happens on unmount/remount?
- If
PersistQueryClientProviderunmounts, it will unsubscribe from persistence syncing (that’s the point of using it). [1] - If it mounts again, it will run the restore again and re-subscribe. (So in most apps, you want this provider mounted once at the app root.)
Manual pattern (if you’re not using PersistQueryClientProvider)
Use explicit restore + subscribe in an effect, and cleanup on unmount:
import { useEffect } from 'react'
import {
persistQueryClientRestore,
persistQueryClientSubscribe,
} from '@tanstack/react-query-persist-client'
function usePersistedQueryClient({ queryClient, persister, buster, maxAge }: any) {
useEffect(() => {
let unsub: (() => void) | undefined
;(async () => {
await persistQueryClientRestore({ queryClient, persister, buster, maxAge })
unsub = persistQueryClientSubscribe({ queryClient, persister, buster })
})()
return () => {
unsub?.()
}
}, [queryClient, persister, buster, maxAge])
}This mirrors what the provider is designed to manage for you. [1]
Source
[1] TanStack Query docs: persistQueryClient / PersistQueryClientProvider lifecycle + restoring behavior (tanstack.dev)
Citations:
🏁 Script executed:
find . -type f \( -name "*.tsx" -o -name "*.ts" \) 2>/dev/nullRepository: TanStack/query
Length of output: 50371
🏁 Script executed:
file $(git ls-files | head -1) 2>/dev/nullRepository: TanStack/query
Length of output: 42
🏁 Script executed:
git diff-tree --no-commit-id --name-only -r HEAD 2>/dev/null | head -20Repository: TanStack/query
Length of output: 40
🌐 Web query:
TanStack query persist client PersistQueryClientProvider.tsx github
💡 Result:
The source for PersistQueryClientProvider lives in the TanStack Query monorepo here (GitHub):
https://github.com/TanStack/query/blob/main/packages/react-query-persist-client/src/PersistQueryClientProvider.tsx
This component is part of @tanstack/react-query-persist-client and is the recommended React wrapper around persistQueryClient so restore/subscribe/unsubscribe happens in the React lifecycle and queries don’t race-fetch while the cache is restoring. [1][2]
Sources: [1], [2]
🏁 Script executed:
git ls-files packages/ 2>/dev/null || echo "Checking without git"; ls -la packages/ 2>/dev/null || echo "No packages dir"Repository: TanStack/query
Length of output: 44438
🏁 Script executed:
rg "useEffect.*didRestore"Repository: TanStack/query
Length of output: 40
🌐 Web query:
TanStack "PersistQueryClientProvider" preact implementation
💡 Result:
There’s no special TanStack Query “preact” build for PersistQueryClientProvider—you use the normal React packages, and run them through Preact’s React-compat layer (preact/compat).
1) Make react resolve to preact/compat
One simple approach is a package alias so anything importing react gets @preact/compat instead (works in Node tooling too): (bret.io)
// package.json
{
"dependencies": {
"preact": "^10.0.0",
"@tanstack/react-query": "^5.0.0",
"@tanstack/react-query-persist-client": "^5.0.0",
"@tanstack/query-sync-storage-persister": "^5.0.0",
"react": "npm:`@preact/compat`@^18.0.0"
}
}(Alternatively, you can do a bundler alias to map react -> preact/compat.)
2) Use PersistQueryClientProvider exactly like in React
PersistQueryClientProvider is provided by @tanstack/react-query-persist-client and is designed to manage restore + subscribe/unsubscribe via the component lifecycle. (tanstack.dev)
localStorage (sync) example (Preact entry)
// main.tsx (or similar)
import { render } from 'preact'
import { QueryClient } from '@tanstack/react-query'
import { PersistQueryClientProvider } from '@tanstack/react-query-persist-client'
import { createSyncStoragePersister } from '@tanstack/query-sync-storage-persister'
import { App } from './app'
const queryClient = new QueryClient({
defaultOptions: {
queries: {
gcTime: 1000 * 60 * 60 * 24, // 24h
},
},
})
const persister = createSyncStoragePersister({
storage: typeof window !== 'undefined' ? window.localStorage : undefined,
})
render(
<PersistQueryClientProvider
client={queryClient}
persistOptions={{ persister }}
>
<App />
</PersistQueryClientProvider>,
document.getElementById('app')!,
)createSyncStoragePersister is the official helper for localStorage/sessionStorage. (tanstack.com)
Citations:
- 1: https://bret.io/blog/2026/simple-tanstack-query-in-preact/?utm_source=openai
- 2: https://tanstack.dev/query/latest/docs/framework/react/plugins/persistQueryClient?utm_source=openai
- 3: https://tanstack.com/query/latest/docs/framework/react/plugins/createSyncStoragePersister?utm_source=openai
🏁 Script executed:
git config --local remote.origin.url 2>/dev/null || git config --global user.name 2>/dev/null || echo "Git config unavailable"Repository: TanStack/query
Length of output: 95
🏁 Script executed:
rg "\.finally\(\(" -t tsx -A 2Repository: TanStack/query
Length of output: 85
🌐 Web query:
TanStack query repository persist client PersistQueryClientProvider
💡 Result:
PersistQueryClientProvider lives in the TanStack Query monorepo under packages/react-query-persist-client (source: packages/react-query-persist-client/src/PersistQueryClientProvider.tsx). [3]
What it does
Use PersistQueryClientProvider (from @tanstack/react-query-persist-client) instead of QueryClientProvider so that:
- the persisted cache is restored asynchronously before queries start fetching (avoids restore/fetch race conditions)
- the persist “subscription” is correctly subscribed/unsubscribed with the React lifecycle. [1]
Install (typical)
@tanstack/react-query-persist-client- a persister, e.g.
@tanstack/query-sync-storage-persister(weblocalStorage) or@tanstack/query-async-storage-persister(AsyncStorage / async). [1][2]
Minimal example (web localStorage)
import { QueryClient } from '@tanstack/react-query'
import { PersistQueryClientProvider } from '@tanstack/react-query-persist-client'
import { createSyncStoragePersister } from '@tanstack/query-sync-storage-persister'
const queryClient = new QueryClient({
defaultOptions: {
queries: {
gcTime: 1000 * 60 * 60 * 24, // important for persistence/hydration
},
},
})
const persister = createSyncStoragePersister({
storage: window.localStorage,
})
export function Root() {
return (
<PersistQueryClientProvider
client={queryClient}
persistOptions={{ persister }}
// optional:
// onSuccess={() => queryClient.resumePausedMutations()}
>
<App />
</PersistQueryClientProvider>
)
}PersistQueryClientProvider props:
- everything
QueryClientProvidersupports, plus:persistOptions(same options aspersistQueryClientminus thequeryClient)onSuccess/onErrorcallbacks for the initial restore. [1]
If you tell me your target (web vs React Native) and TanStack Query major version (v4 vs v5), I can tailor the exact packages/imports.
🌐 Web query:
site:github.com "persistQueryClientRestore" preact
💡 Result:
Your GitHub query (site:github.com "persistQueryClientRestore" preact) doesn’t surface any Preact-specific usage/examples of persistQueryClientRestore. Instead, persistQueryClientRestore is a TanStack Query persistence utility (it restores/hydrates a previously persisted cache into a QueryClient) and is not inherently tied to React/Preact at the function level. [1]
If you’re looking for “Preact + TanStack Query” support, TanStack Query has an open “Feat: Preact Adapter” PR (#9935), which is the most directly relevant GitHub hit connecting Preact to the TanStack Query ecosystem. [2]
Sources: [1], [2]
🏁 Script executed:
find . -type f 2>/dev/null | wc -l && find . -type f 2>/dev/null | head -50Repository: TanStack/query
Length of output: 3459
🏁 Script executed:
wc -l packages/preact-query-persist-client/src/PersistQueryClientProvider.tsxRepository: TanStack/query
Length of output: 132
🏁 Script executed:
cat packages/react-query-persist-client/src/PersistQueryClientProvider.tsxRepository: TanStack/query
Length of output: 1803
🏁 Script executed:
# Let's check for any unmount handling or cleanup in both files
rg "return.*=>|useEffect.*cleanup" packages/react-query-persist-client/src/PersistQueryClientProvider.tsx packages/preact-query-persist-client/src/PersistQueryClientProvider.tsxRepository: TanStack/query
Length of output: 40
🏁 Script executed:
grep -A 10 "unmount\|cleanup\|cancel" packages/react-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsxRepository: TanStack/query
Length of output: 40
Guard setIsRestoring(false) against state updates after unmount.
The persistQueryClientRestore promise chain executes its finally block and calls setIsRestoring(false) even if the component unmounts before the restore settles. This causes a React "Cannot update an unmounted component" warning. Add a mounted flag to the cleanup function:
♻️ Suggested fix
React.useEffect(() => {
+ let isMounted = true
const options = {
...refs.current.persistOptions,
queryClient: props.client,
}
if (!didRestore.current) {
didRestore.current = true
persistQueryClientRestore(options)
.then(() => refs.current.onSuccess?.())
.catch(() => refs.current.onError?.())
.finally(() => {
+ if (isMounted) {
setIsRestoring(false)
+ }
})
}
- return isRestoring ? undefined : persistQueryClientSubscribe(options)
+ return () => {
+ isMounted = false
+ return isRestoring ? undefined : persistQueryClientSubscribe(options)
+ }
}, [props.client, isRestoring])🤖 Prompt for AI Agents
In `@packages/preact-query-persist-client/src/PersistQueryClientProvider.tsx`
around lines 33 - 48, The finally handler calls setIsRestoring(false) even after
unmount, causing state updates on unmounted component; inside the useEffect that
references didRestore, refs.current, persistQueryClientRestore, isRestoring and
persistQueryClientSubscribe, add a mounted flag (e.g., let mounted = true) and
set mounted = false in the cleanup, then only call setIsRestoring(false) in the
persistQueryClientRestore promise chain if mounted is true; ensure the cleanup
still returns persistQueryClientSubscribe(options) when not restoring and the
mounted flag prevents setIsRestoring from running after unmount.
| import type { UserConfig as ViteUserConfig } from 'vite' | ||
|
|
||
| import packageJson from './package.json' |
There was a problem hiding this comment.
Fix import order: ./package.json should be imported before the type import from vite.
ESLint flags this as an import/order violation.
Proposed fix
-import { defineConfig } from 'vitest/config'
-import type { UserConfig as ViteUserConfig } from 'vite'
-
-import packageJson from './package.json'
+import { defineConfig } from 'vitest/config'
+import packageJson from './package.json'
+import type { UserConfig as ViteUserConfig } from 'vite'🧰 Tools
🪛 ESLint
[error] 5-5: ./package.json import should occur before type import of vite
(import/order)
🤖 Prompt for AI Agents
In `@packages/preact-query-persist-client/vite.config.ts` around lines 3 - 5,
Reorder the imports so the package JSON import appears before the Vite type
import to satisfy import/order; move "import packageJson from './package.json'"
above "import type { UserConfig as ViteUserConfig } from 'vite'"; ensure symbols
packageJson and ViteUserConfig remain unchanged and that no other code is
affected by the swap.
There was a problem hiding this comment.
Pull request overview
This PR adds the @tanstack/preact-query-persist-client package, providing Preact framework support for query client persistence functionality. This fills a gap in the TanStack Query ecosystem by bringing persist capabilities to Preact users, similar to what already exists for React, Solid, and Angular.
Changes:
- Adds new
@tanstack/preact-query-persist-clientpackage with PersistQueryClientProvider component - Implements Preact-specific bindings for query persistence using hooks and the query-persist-client-core
- Adds documentation linking to React plugin docs with appropriate text replacements
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds dependencies for the new preact-query-persist-client package |
| packages/preact-query-persist-client/package.json | Package manifest defining dependencies, scripts, and build configuration |
| packages/preact-query-persist-client/src/PersistQueryClientProvider.tsx | Main component implementing persistence state management using Preact hooks |
| packages/preact-query-persist-client/src/index.ts | Re-exports core functionality and the PersistQueryClientProvider |
| packages/preact-query-persist-client/vite.config.ts | Vitest configuration for testing with Preact preset |
| packages/preact-query-persist-client/tsup.config.ts | Build configuration using modern and legacy targets |
| packages/preact-query-persist-client/tsconfig*.json | TypeScript configurations for development, production, and legacy builds |
| packages/preact-query-persist-client/test-setup.ts | Test environment setup with Preact Testing Library |
| packages/preact-query-persist-client/eslint.config.js | ESLint configuration extending root config with Preact-specific rules |
| packages/preact-query-persist-client/root.*.js | Symlinks to shared build and lint configurations |
| docs/framework/preact/plugins/*.md | Documentation files referencing React plugin docs with text replacement |
| docs/config.json | Adds Preact plugins section to documentation navigation |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/preact-query-persist-client/vite.config.ts:33
- The vite config is missing the retry configuration that exists in the preact-query package vite config. Consider adding
retry: process.env.CI ? 3 : 0,to the test configuration to match the pattern used in packages/preact-query/vite.config.ts.
test: {
name: packageJson.name,
dir: './src',
watch: false,
environment: 'jsdom',
setupFiles: ['test-setup.ts'],
coverage: {
enabled: true,
provider: 'istanbul',
include: ['src/**/*'],
exclude: ['src/__tests__/**'],
},
typecheck: { enabled: true },
restoreMocks: true,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const PersistQueryClientProvider = ({ | ||
| children, | ||
| persistOptions, | ||
| onSuccess, | ||
| onError, | ||
| ...props | ||
| }: PersistQueryClientProviderProps): VNode => { | ||
| const [isRestoring, setIsRestoring] = useState(true) | ||
| const refs = useRef({ persistOptions, onSuccess, onError }) | ||
| const didRestore = useRef(false) | ||
|
|
||
| useEffect(() => { | ||
| refs.current = { persistOptions, onSuccess, onError } | ||
| }) | ||
|
|
||
| useEffect(() => { | ||
| const options = { | ||
| ...refs.current.persistOptions, | ||
| queryClient: props.client, | ||
| } | ||
| if (!didRestore.current) { | ||
| didRestore.current = true | ||
| persistQueryClientRestore(options) | ||
| .then(() => refs.current.onSuccess?.()) | ||
| .catch(() => refs.current.onError?.()) | ||
| .finally(() => { | ||
| setIsRestoring(false) | ||
| }) | ||
| } | ||
| return isRestoring ? undefined : persistQueryClientSubscribe(options) | ||
| }, [props.client, isRestoring]) | ||
|
|
||
| return ( | ||
| <QueryClientProvider {...props}> | ||
| <IsRestoringProvider value={isRestoring}>{children}</IsRestoringProvider> | ||
| </QueryClientProvider> | ||
| ) | ||
| } |
There was a problem hiding this comment.
The PersistQueryClientProvider component lacks test coverage. Other persist client packages in the monorepo (react-query-persist-client and solid-query-persist-client) have comprehensive test suites that verify functionality like cache restoration, error handling, success/error callbacks, and StrictMode behavior. Please add similar test coverage for the Preact implementation.
|
Great stuff with this PR as well! Can we implement the test suite for this? |
|
@theVedanta not sure why you closed this pr, could you provide your reasoning? |
I'm sorry 😭 an accident, reopened! |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
packages/preact-query-persist-client/src/__tests__/testPersistProvider.tsx (2)
14-20:clientandpersistOptionstyped asany— use the actual prop types
anydefeats type-checking and will silently permit malformed props. ImportPersistQueryClientProviderProps(or equivalent) from the actual library source and derive thePropstype from it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/preact-query-persist-client/src/__tests__/testPersistProvider.tsx` around lines 14 - 20, Replace the loose any types for client and persistOptions by importing the real provider prop type (PersistQueryClientProviderProps) from the library and deriving Props from it; update the local Props declaration to use PersistQueryClientProviderProps for client and persistOptions (and keep children optional as ComponentChildren if needed) so the test types match the actual component API and get proper type-checking.
43-50: In-flight restore promise is not cancelled on unmountIf the component unmounts before
persistQueryClientRestoreresolves, the.finally(() => setIsRestoring(false))callback still fires, updating state on an unmounted component. Add anisMountedguard or use anAbortControllerpattern to avoid this.♻️ Proposed fix
useEffect(() => { const options = { ...refs.current.persistOptions, queryClient: props.client, } + let cancelled = false if (!didRestore.current) { didRestore.current = true persistQueryClientRestore(options) .then(() => refs.current.onSuccess?.()) .catch(() => refs.current.onError?.()) .finally(() => { - setIsRestoring(false) + if (!cancelled) setIsRestoring(false) }) } - return isRestoring ? undefined : persistQueryClientSubscribe(options) + return () => { + cancelled = true + if (!isRestoring) persistQueryClientSubscribe(options)() + } }, [props.client, isRestoring])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/preact-query-persist-client/src/__tests__/testPersistProvider.tsx` around lines 43 - 50, The restore promise currently calls refs.current callbacks and setIsRestoring in its .then/.catch/.finally even if the component unmounts; add an isMounted guard (or use AbortController) inside the effect where didRestore and persistQueryClientRestore are used: create a let isMounted = true at start, in the unmount cleanup set isMounted = false (or call controller.abort()), then only invoke refs.current.onSuccess(), refs.current.onError(), and setIsRestoring(false) if isMounted is still true; reference didRestore.current, persistQueryClientRestore(options), refs.current.onSuccess/onError, and setIsRestoring when adding the guard/cancellation to prevent state updates after unmount.packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx (1)
41-120: Only one test case covers the provider — expand the test suiteThe PR reviewer
theVedantaalready requested a fuller test suite. Consider adding coverage for:
- Successful restore with
onSuccesscallback- Restore failure with
onErrorcallback- Stale/expired cache being discarded (client cleared, not restored)
persistQueryClientSubscribereceiving cache updates post-restoreHappy to generate additional test cases if you'd like to open a tracking issue or expand this file directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx` around lines 41 - 120, Add multiple new tests in PersistQueryClientProvider.test.tsx to cover restore success and failure, stale cache discard, and subscription updates: create tests that reuse QueryClient, createMockPersister and persistQueryClientSave to simulate a successful restore and assert an onSuccess spy is called and UI shows restored data; create a test where the persister rejects and assert an onError spy is called and client stays empty; add a test that writes an expired/stale entry, clears the client before restore and assert no restore occurs; and add a test that uses persistQueryClientSubscribe (or the persister mock subscribe) to emit a cache update after restore and assert the component receives the updated data — use the existing Page component pattern, vi.useFakeTimers/advanceTimersByTimeAsync, and inspect the states array and rendered output to validate fetchStatus transitions and final data in each case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx`:
- Around line 3-17: Reorder the import statements so external packages come
first, workspace-relative imports next, and local relative imports last: place
'@testing-library/jest-dom/vitest', '@testing-library/preact' and 'vitest'
imports before the '../../../...' module imports (PersistedClient, Persister,
persistQueryClientSave, notifyManager, UseQueryResult, QueryClient, useQuery),
and then the local imports ('./testPersistProvider', './testUtils'); keep a
blank line between each group and preserve or alphabetize within groups to
satisfy import/order rules.
- Line 16: Tests import a local reimplementation (testPersistProvider) instead
of the real PersistQueryClientProvider, so the test suite bypasses the library's
persistence/restore/subscribe and IsRestoringProvider behavior; update the test
import to use the actual component implementation (PersistQueryClientProvider
from src/PersistQueryClientProvider.tsx) so tests exercise the real
restore/subscribe logic and IsRestoringProvider integration, and remove or adapt
any local stubbing in PersistQueryClientProvider.test.tsx that masks the real
component's behavior.
In `@packages/preact-query-persist-client/src/__tests__/testPersistProvider.tsx`:
- Around line 3-12: Reorder the top-level imports to satisfy ESLint
import/order: move the external import from 'preact/hooks' (useEffect, useRef,
useState) before any workspace-relative imports so external modules come first;
keep the named symbols intact (persistQueryClientRestore,
persistQueryClientSubscribe, IsRestoringProvider, QueryClientProvider,
ComponentChildren) and ensure the import lines for 'preact/hooks' appear above
the workspace-relative paths to resolve the lint failure.
- Around line 22-60: The test helper currently reimplements the real
PersistQueryClientProvider (including useState/useRef/useEffect and calls to
persistQueryClientRestore/persistQueryClientSubscribe), so replace that
duplicated implementation in testPersistProvider.tsx with a direct re-export of
the real component: import the actual PersistQueryClientProvider from the
library module where it is implemented and re-export it (preserving the named
export), so tests like PersistQueryClientProvider.test.tsx and
use-queries-with-persist.test.tsx exercise the shipped
PersistQueryClientProvider instead of the local copy.
In
`@packages/preact-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx`:
- Around line 28-42: The test sets window.localStorage via Object.defineProperty
but omits configurable: true, causing the property to be non-configurable and
breaking subsequent beforeEach calls; update the Object.defineProperty call that
defines window.localStorage in the test (the block that sets
getItem/setItem/removeItem/clear) to include configurable: true in the
descriptor so the property can be redefined or cleaned up between tests.
- Around line 3-14: Reorder the imports in use-queries-with-persist.test.tsx to
satisfy import/order: place external packages first (e.g.
'@testing-library/jest-dom/vitest', '@testing-library/preact', 'vitest'), then
workspace-relative imports from query packages (e.g. QueryObserverResult,
notifyManager from '../../../query-core/src', PersistedClient/Persister from
'../../../query-persist-client-core/src', QueryClient/useQueries from
'../../../preact-query/src'), and finally local or relative hooks/util imports
(e.g. useCallback from 'preact/hooks'); keep each import group together and
preserve the existing imported symbols (act, cleanup, render,
QueryObserverResult, notifyManager, PersistedClient, Persister, QueryClient,
useQueries, useCallback, afterEach, beforeEach, describe, expect, it, vi) to fix
the ESLint import/order failure.
---
Nitpick comments:
In
`@packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx`:
- Around line 41-120: Add multiple new tests in
PersistQueryClientProvider.test.tsx to cover restore success and failure, stale
cache discard, and subscription updates: create tests that reuse QueryClient,
createMockPersister and persistQueryClientSave to simulate a successful restore
and assert an onSuccess spy is called and UI shows restored data; create a test
where the persister rejects and assert an onError spy is called and client stays
empty; add a test that writes an expired/stale entry, clears the client before
restore and assert no restore occurs; and add a test that uses
persistQueryClientSubscribe (or the persister mock subscribe) to emit a cache
update after restore and assert the component receives the updated data — use
the existing Page component pattern, vi.useFakeTimers/advanceTimersByTimeAsync,
and inspect the states array and rendered output to validate fetchStatus
transitions and final data in each case.
In `@packages/preact-query-persist-client/src/__tests__/testPersistProvider.tsx`:
- Around line 14-20: Replace the loose any types for client and persistOptions
by importing the real provider prop type (PersistQueryClientProviderProps) from
the library and deriving Props from it; update the local Props declaration to
use PersistQueryClientProviderProps for client and persistOptions (and keep
children optional as ComponentChildren if needed) so the test types match the
actual component API and get proper type-checking.
- Around line 43-50: The restore promise currently calls refs.current callbacks
and setIsRestoring in its .then/.catch/.finally even if the component unmounts;
add an isMounted guard (or use AbortController) inside the effect where
didRestore and persistQueryClientRestore are used: create a let isMounted = true
at start, in the unmount cleanup set isMounted = false (or call
controller.abort()), then only invoke refs.current.onSuccess(),
refs.current.onError(), and setIsRestoring(false) if isMounted is still true;
reference didRestore.current, persistQueryClientRestore(options),
refs.current.onSuccess/onError, and setIsRestoring when adding the
guard/cancellation to prevent state updates after unmount.
packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx
Outdated
Show resolved
Hide resolved
packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx
Show resolved
Hide resolved
packages/preact-query-persist-client/src/__tests__/testPersistProvider.tsx
Show resolved
Hide resolved
packages/preact-query-persist-client/src/__tests__/testPersistProvider.tsx
Show resolved
Hide resolved
packages/preact-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx
Show resolved
Hide resolved
packages/preact-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/preact-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx (1)
45-59:queryClientshould be destroyed inafterEachto prevent resource leaks.
queryClientis created inside theit()body, soafterEachcan't reach it. Scoping it to thedescribelevel and destroying it inafterEachis the safer, more scalable pattern — especially as additional tests are expected per the PR comments.♻️ Proposed refactor
describe('useQueries with persist and memoized combine (preact)', () => { const storage: Record<string, string> = {} + let queryClient: QueryClient beforeEach(() => { vi.useFakeTimers() + queryClient = new QueryClient({ + defaultOptions: { + queries: { + staleTime: 30_000, + gcTime: 1000 * 60 * 60 * 24, + }, + }, + }) Object.defineProperty(window, 'localStorage', { // ... }) }) afterEach(() => { cleanup() + queryClient.destroy() vi.useRealTimers() Object.keys(storage).forEach((key) => delete storage[key]) }) it('updates UI when combine is memoized with persisted results', async () => { - const queryClient = new QueryClient({ - defaultOptions: { - queries: { - staleTime: 30_000, - gcTime: 1000 * 60 * 60 * 24, - }, - }, - }) // rest of test unchanged }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/preact-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx` around lines 45 - 59, The test creates a QueryClient inside the it() block which prevents afterEach from cleaning it up; move the QueryClient declaration to describe-level scope (declare let queryClient) and instantiate it in beforeEach, then call queryClient.destroy() in afterEach alongside cleanup() and vi.useRealTimers() so the QueryClient is always destroyed and avoids resource leaks; update references in the test to use the describe-scoped queryClient.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@packages/preact-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx`:
- Around line 28-42: The test's Object.defineProperty call that stubs
window.localStorage (the descriptor defined inline in the test file) is missing
configurable: true, which makes the property non-redefinable and causes
subsequent beforeEach runs to throw; update the descriptor passed to
Object.defineProperty(window, 'localStorage', { ... }) to include configurable:
true so the stub can be redefined/cleared across tests (keep existing
getItem/setItem/removeItem/clear and writable: true).
- Around line 3-17: Import ordering is wrong per ESLint import/order; reorder
imports so external packages come first, then internal project packages, then
local test files. Specifically, place '@testing-library/jest-dom/vitest',
'@testing-library/preact', 'preact/hooks', and 'vitest' imports (act, cleanup,
render, useCallback, afterEach, beforeEach, describe, expect, it, vi) before the
internal imports from '../../../query-core/src' (QueryObserverResult,
notifyManager), '../../../query-persist-client-core/src' (PersistedClient,
Persister), and '../../../preact-query/src' (QueryClient, useQueries), and keep
the local test imports PersistQueryClientProvider and sleep last; adjust any
type vs value import grouping to match the project's import/order rules so
ESLint passes.
---
Nitpick comments:
In
`@packages/preact-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx`:
- Around line 45-59: The test creates a QueryClient inside the it() block which
prevents afterEach from cleaning it up; move the QueryClient declaration to
describe-level scope (declare let queryClient) and instantiate it in beforeEach,
then call queryClient.destroy() in afterEach alongside cleanup() and
vi.useRealTimers() so the QueryClient is always destroyed and avoids resource
leaks; update references in the test to use the describe-scoped queryClient.
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:sherif,test:knip,tes... |
❌ Failed | 1m 4s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 7s | View ↗ |
☁️ Nx Cloud last updated this comment at 2026-02-23 06:08:57 UTC
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx (1)
31-33:asynckeyword onrestoreClientis redundant.The function body already returns a
Promisevia.then(); theasyncwrapper adds nothing.♻️ Proposed fix
- async restoreClient() { - return sleep(10).then(() => storedState) - }, + restoreClient() { + return sleep(10).then(() => storedState) + },Based on learnings, when a function body has no side effects, prefer the
.then()pattern for conciseness over mixingasyncwith.then().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx` around lines 31 - 33, The restoreClient function is marked async but immediately returns a Promise via .then(), so remove the redundant async keyword on restoreClient (the function in the test suite named restoreClient) or alternatively replace the .then() chain with an await if you prefer async/await consistency; either remove async to keep the concise .then() pattern or refactor the body to use await sleep(10) then return storedState so async/await is used properly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx`:
- Line 16: The test imports the reimplemented testPersistProvider instead of the
real component, so replace the import to use the actual
PersistQueryClientProvider implementation; locate the test file import line that
references './testPersistProvider' and change it to import
PersistQueryClientProvider from the library's real module (the same symbol name
PersistQueryClientProvider) so tests exercise src/PersistQueryClientProvider.tsx
and will cover restore/subscribe lifecycle and IsRestoringProvider integration.
- Around line 5-13: Reorder the import statements so external packages come
first, then workspace-relative imports, then local relative imports: move
imports like "act, cleanup, render" from '@testing-library/preact' (and any
"vitest" imports) to the top, followed by workspace imports such as
"PersistedClient, Persister", "persistQueryClientSave", "notifyManager",
"UseQueryResult", and "QueryClient, useQuery" from the '../../../...' modules;
ensure the import order groups types and values consistently and update the
file's import block (identifiers to look for: persistQueryClientSave,
notifyManager, PersistedClient, Persister, UseQueryResult, QueryClient,
useQuery, act, cleanup, render) so ESLint import/order no longer errors.
In
`@packages/preact-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx`:
- Around line 28-42: The test's Object.defineProperty call that mocks
window.localStorage (the block defining getItem/setItem/removeItem/clear) is
missing configurable: true, which will cause TypeError on subsequent
redefinitions; update that Object.defineProperty invocation to include
configurable: true in the property descriptor so window.localStorage can be
redefined across tests (keep the existing getItem/setItem/removeItem/clear
implementations and writable: true).
- Around line 5-14: The import/order ESLint errors are caused by mixing external
and workspace-relative imports; reorder imports so external packages (vitest,
`@testing-library/preact`, preact/hooks) come first, then workspace-relative
modules (the '../../../query-core/src' imports,
'../../../query-persist-client-core/src' types, and '../../../preact-query/src'
imports). Specifically, move "vitest" and "@testing-library/preact" and
"preact/hooks" imports above the QueryCore/PersistClient/QueryClient imports and
keep type-only imports (e.g., QueryObserverResult, PersistedClient, Persister)
grouped with their workspace modules; adjust import grouping in
use-queries-with-persist.test.tsx to satisfy import/order.
In `@packages/preact-query-persist-client/src/PersistQueryClientProvider.tsx`:
- Around line 1-2: The import ordering violates ESLint import/order: move the
type import "import type { VNode } from 'preact'" so it sits after any imports
from "@tanstack/preact-query"; locate the top of PersistQueryClientProvider.tsx
where "useEffect, useRef, useState" are imported and reorder so
"@tanstack/preact-query" imports come before the "VNode" type import to satisfy
the rule.
- Around line 39-54: In PersistQueryClientProvider's useEffect the finally
handler of persistQueryClientRestore calls setIsRestoring(false) even if the
component has unmounted; add an isMounted boolean (e.g., let isMounted = true)
at the start of the effect, set isMounted = false in the cleanup, and only call
setIsRestoring(false) inside the finally if isMounted is true; ensure the
cleanup still returns persistQueryClientSubscribe(options) when mounted and that
didRestore/current logic remains intact (refs.current.persistOptions,
persistQueryClientRestore, setIsRestoring, persistQueryClientSubscribe).
---
Nitpick comments:
In
`@packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx`:
- Around line 31-33: The restoreClient function is marked async but immediately
returns a Promise via .then(), so remove the redundant async keyword on
restoreClient (the function in the test suite named restoreClient) or
alternatively replace the .then() chain with an await if you prefer async/await
consistency; either remove async to keep the concise .then() pattern or refactor
the body to use await sleep(10) then return storedState so async/await is used
properly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx (1)
23-39:createMockPersisterduplicates the shared utility inquery-persist-client-core
packages/query-persist-client-core/src/__tests__/utils.tsalready exports an identicalcreateMockPersister. Consider importing from there instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx` around lines 23 - 39, The test file duplicates the createMockPersister implementation — replace the local createMockPersister with an import from the shared test utility in query-persist-client-core to avoid duplication; update the test imports to import { createMockPersister } (and any needed types like Persister or PersistedClient) from packages/query-persist-client-core/src/__tests__/utils.ts and remove the local factory function so tests use the shared createMockPersister implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/preact-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx`:
- Line 17: The test is importing a local test double
`PersistQueryClientProvider` from `./testPersistProvider` instead of the real
shipped implementation; replace that import to use the actual
`PersistQueryClientProvider` implementation (the one originally in
`src/PersistQueryClientProvider.tsx`) so the test exercises the real restoration
lifecycle, `persistQueryClientSubscribe` teardown, and `IsRestoringProvider`
integration rather than a line-for-line copy; update the import to reference the
real module and remove the test provider fixture so tests run against
`PersistQueryClientProvider`.
---
Duplicate comments:
In
`@packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx`:
- Around line 5-14: The import ordering in PersistQueryClientProvider.test.tsx
is incorrect: workspace-relative imports (e.g., symbols PersistedClient,
Persister, persistQueryClientSave, notifyManager, UseQueryResult, QueryClient,
useQuery) are placed before external package imports (e.g., act, cleanup, render
from `@testing-library/preact` and vitest imports afterEach, beforeEach, describe,
expect, test, vi), causing ESLint import/order violations—fix by grouping and
ordering imports so external packages come first (all `@testing-library/preact`
and vitest imports), followed by internal/workspace-relative imports (the
../../../... modules like persistQueryClientSave, notifyManager,
QueryClient/useQuery, types PersistedClient/UseQueryResult), preserving existing
specifiers and relative import paths.
- Line 17: The test is importing a local duplicate (testPersistProvider) instead
of the real component, so update the import in
PersistQueryClientProvider.test.tsx to import PersistQueryClientProvider (and
related exports like IsRestoringProvider) from the actual implementation
(src/PersistQueryClientProvider.tsx) and remove usage of testPersistProvider;
then ensure the tests exercise the real component's restoration, subscription,
and IsRestoringProvider wiring (e.g., mount PersistQueryClientProvider, trigger
restore/subscribe flows, and assert IsRestoringProvider behavior) so regressions
in the real implementation are caught.
In
`@packages/preact-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx`:
- Around line 28-42: The test sets window.localStorage via Object.defineProperty
without setting configurable: true, which makes the property non-redefinable and
will cause "TypeError: Cannot redefine property: localStorage" in later tests;
update the Object.defineProperty call that defines window.localStorage (the
block referencing window, localStorage, getItem/setItem/removeItem/clear) to
include configurable: true in the property descriptor so subsequent beforeEach
blocks can redefine or restore localStorage safely.
- Around line 5-14: The import order in this test file violates ESLint
import/order by placing workspace-relative imports before external packages;
reorder the top imports so all external modules (e.g.,
'@testing-library/preact', 'vitest', 'preact/hooks') appear before
workspace/local imports (e.g., '../../../query-core/src' imports like
QueryObserverResult and notifyManager, '../../../query-persist-client-core/src'
types PersistedClient and Persister, and '../../../preact-query/src' exports
QueryClient and useQueries), keeping named symbols (act, cleanup, render,
useCallback, afterEach, beforeEach, describe, expect, it, vi) with their
respective packages and preserving existing specifiers and spacing.
---
Nitpick comments:
In
`@packages/preact-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx`:
- Around line 23-39: The test file duplicates the createMockPersister
implementation — replace the local createMockPersister with an import from the
shared test utility in query-persist-client-core to avoid duplication; update
the test imports to import { createMockPersister } (and any needed types like
Persister or PersistedClient) from
packages/query-persist-client-core/src/__tests__/utils.ts and remove the local
factory function so tests use the shared createMockPersister implementation.
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' | ||
| import { sleep } from '@tanstack/query-test-utils' | ||
|
|
||
| import { PersistQueryClientProvider } from './testPersistProvider' |
There was a problem hiding this comment.
Tests cover a re-implementation instead of the real PersistQueryClientProvider
./testPersistProvider mirrors src/PersistQueryClientProvider.tsx line-for-line. The restoration lifecycle, persistQueryClientSubscribe teardown, and IsRestoringProvider integration exercised in this test all run against the copy, not the shipped code. Import from the actual implementation instead:
-import { PersistQueryClientProvider } from './testPersistProvider'
+import { PersistQueryClientProvider } from '../PersistQueryClientProvider'📝 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.
| import { PersistQueryClientProvider } from './testPersistProvider' | |
| import { PersistQueryClientProvider } from '../PersistQueryClientProvider' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/preact-query-persist-client/src/__tests__/use-queries-with-persist.test.tsx`
at line 17, The test is importing a local test double
`PersistQueryClientProvider` from `./testPersistProvider` instead of the real
shipped implementation; replace that import to use the actual
`PersistQueryClientProvider` implementation (the one originally in
`src/PersistQueryClientProvider.tsx`) so the test exercises the real restoration
lifecycle, `persistQueryClientSubscribe` teardown, and `IsRestoringProvider`
integration rather than a line-for-line copy; update the import to reference the
real module and remove the test provider fixture so tests run against
`PersistQueryClientProvider`.
2 participants
🎯 Changes
This adds the persist plugin, similar to how other integrations have the plugin.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
This is the initial release of the package
Summary by CodeRabbit
New Features
Documentation