Open
Conversation
🦋 Changeset detectedLatest commit: afced99 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
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:
📝 WalkthroughWalkthroughAdds a new persister method Changes
Sequence Diagram(s)sequenceDiagram
participant QC as QueryClient
participant P as Persister
participant S as Storage
QC->>P: removeQueries(filters)
P->>S: entries()
loop for each (key, value)
S-->>P: (key, value)
P->>P: try deserialize(value)
alt deserialize OK and matches filters
P->>S: removeItem(key)
else deserialize fails or doesn't match
P->>P: skip entry (maybe remove if expired/busted)
end
end
P-->>QC: Promise resolves
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
github-actions
bot
added
documentation
|
View your CI Pipeline Execution ↗ for commit afced99
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/framework/react/plugins/createPersister.md (1)
131-132: Tighten wording for clarity and terminology consistency.Line 131 should use “persister” and “needs to be removed separately” for clearer English.
Proposed wording cleanup
-When using `queryClient.removeQueries` the data is kept in the persistor and to be removed separately. +When using `queryClient.removeQueries`, the data remains in the persister and needs to be removed separately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/framework/react/plugins/createPersister.md` around lines 131 - 132, Update the sentence that starts with "When using `queryClient.removeQueries`" to use the term "persister" and the phrase "needs to be removed separately" for clarity and consistency; specifically replace the current wording so it reads something like: "When using `queryClient.removeQueries` the data is kept in the persister and needs to be removed separately. This function can be used to remove queries that are currently stored by the persister." Ensure both occurrences of "persistor" (if present) are changed to "persister" and adjust punctuation as needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/framework/react/plugins/createPersister.md`:
- Around line 129-133: The docs incorrectly show removeQueries(queryClient:
QueryClient, filters) but the real API is removeQueries(filters); update the
documentation for the function signature to remove the QueryClient parameter
(use removeQueries(filters): Promise<void>) and adjust the surrounding
description to explain that the persister's removeQueries accepts only filters
and will remove stored queries accordingly; reference the removeQueries function
name in the docs so readers see the corrected signature.
In `@packages/query-persist-client-core/src/createPersister.ts`:
- Around line 329-331: The dev error message inside removeQueries incorrectly
mentions "Restoration of all stored entries" — update the thrown Error in
createPersister.ts (the branch used by removeQueries) to state that removal of
stored entries is not possible without the storage implementing an `entries`
method/ability to iterate over storage items, keeping the same context and
phrasing but replacing "Restoration" with "Removal" and referencing `entries`.
- Line 325: The call to storage.removeItem(key) inside removeQueries is not
awaited, so async storage implementations may not finish removal before
removeQueries resolves; update the removeQueries function to await the
MaybePromise returned by storage.removeItem(key) (i.e., change
storage.removeItem(key) to await storage.removeItem(key)) so behavior matches
the other awaited removeItem usages in this file (see other calls at lines where
removeItem is awaited) and ensure removeQueries returns only after the removal
completes.
---
Nitpick comments:
In `@docs/framework/react/plugins/createPersister.md`:
- Around line 131-132: Update the sentence that starts with "When using
`queryClient.removeQueries`" to use the term "persister" and the phrase "needs
to be removed separately" for clarity and consistency; specifically replace the
current wording so it reads something like: "When using
`queryClient.removeQueries` the data is kept in the persister and needs to be
removed separately. This function can be used to remove queries that are
currently stored by the persister." Ensure both occurrences of "persistor" (if
present) are changed to "persister" and adjust punctuation as needed.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/framework/react/plugins/createPersister.mdpackages/query-persist-client-core/src/__tests__/createPersister.test.tspackages/query-persist-client-core/src/createPersister.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/fifty-zebras-stay.md:
- Around line 1-5: The changeset incorrectly marks
'@tanstack/query-persist-client-core' as a major bump even though the only
change is adding an additive method removeQueries to the
experimental_createQueryPersister return value; update the changeset to use a
minor version bump instead of major so this non-breaking addition is released as
a minor version.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/query-persist-client-core/src/createPersister.ts`:
- Around line 312-326: The loop currently uses key.startsWith(prefix) and calls
deserialize(value) unguarded which can process unrelated keys and throw; narrow
the key filter to only the persisted-query namespace (e.g. require the key to
start with `${prefix}-persisted-query-` or otherwise validate the
persisted-query key format before deserializing) and wrap the per-entry
deserialize call in a try/catch so a bad entry is skipped instead of aborting
the whole loop; only call storage.removeItem(key) for entries that successfully
deserialize and pass the existing queryKey/exact checks (refer to variables key,
prefix, deserialize, persistedQuery, queryKey, exact, partialMatchKey, hashKey,
and storage.removeItem).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/fifty-zebras-stay.mddocs/framework/react/plugins/createPersister.mdpackages/query-persist-client-core/src/createPersister.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .changeset/fifty-zebras-stay.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/query-persist-client-core/src/createPersister.ts`:
- Around line 319-337: The deserialization catch block in removeQueries (use of
deserialize, key, storageKeyPrefix, storage.removeItem) currently just continues
and leaves corrupted entries when no queryKey filter is provided; change the
catch to remove the item when queryKey is not provided: inside the catch for
deserialize(value) check if queryKey is falsy/undefined and call
storage.removeItem(key) before continuing, otherwise keep the existing continue
behavior so filtered removal still skips malformed entries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
This adds a
removeQueriesmethod to theexperimental_createQueryPersisterSee also #9189
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Documentation
Tests
Chores