fix(createQueryPersister): add option to change behavior of refetch after restore #9745
Conversation
🦋 Changeset detectedLatest commit: a462859 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 |
github-actions
bot
added
documentation
WalkthroughAdds a new StoragePersisterOptions.refetchOnRestore (boolean | 'always'), defaults it to true, updates restoration logic to conditionally trigger query.fetch(), adds tests covering the behaviors, and updates React/Vue docs and Vue AsyncStorage signatures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Persister
participant Storage
participant QueryCache
participant Query
participant Network
App->>Persister: init(refetchOnRestore)
Persister->>Storage: read persisted state
Storage-->>Persister: persisted queries
Persister->>QueryCache: restore queries
alt refetchOnRestore == 'always'
Note right of Persister: Always trigger fetch
Persister->>Query: fetch()
Query->>Network: request
Network-->>Query: response
else refetchOnRestore == true
Note right of Persister: Fetch only if stale
Persister->>Query: isStale?
alt stale
Persister->>Query: fetch()
Query->>Network: request
Network-->>Query: response
else fresh
Note right of Query: No fetch
end
else refetchOnRestore == false
Note right of Query: No fetch on restore
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit a462859
☁️ Nx Cloud last updated this comment at |
|
Sizes for commit a462859:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/framework/react/plugins/createPersister.md(2 hunks)docs/framework/vue/plugins/createPersister.md(4 hunks)packages/query-persist-client-core/src/__tests__/createPersister.test.ts(1 hunks)packages/query-persist-client-core/src/createPersister.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (8)
docs/framework/react/plugins/createPersister.md (1)
172-178: Documentation is clear and accurate.The documentation effectively describes the three behavioral modes of
refetchOnRestoreand their effects on query restoration.packages/query-persist-client-core/src/createPersister.ts (3)
65-71: JSDoc is clear and accurate.The documentation properly describes all three modes of operation and specifies the default value.
106-106: Default value is correctly set.The default of
truealigns with the documented behavior and provides sensible out-of-the-box refetch semantics.
215-220: Implementation correctly handles all refetch modes.The conditional logic properly implements the three behaviors:
'always': refetches unconditionallytrue: refetches only when stalefalse: never refetchesdocs/framework/vue/plugins/createPersister.md (4)
36-36: Generic reference is appropriate for Vue documentation.Removing the React Native-specific reference makes the documentation more framework-agnostic and suitable for Vue users.
40-40: GitHub issue reference adds helpful context.Linking to the issue provides users with additional context about the limitation with
setQueryData()operations.
69-124: Utilities documentation improves completeness.Adding the "Additional utilities" section brings the Vue documentation to parity with the React documentation and provides essential information about helper functions.
169-175: Documentation accurately reflects the implementation.The
refetchOnRestoreoption is properly documented with all three modes, and theAsyncStorageinterface is correctly shown as generic with optionalentries()support, matching the TypeScript implementation.Also applies to: 182-186, 198-198
| test('should restore item from the storage and refetch when `refetchOnRestore` is set to `always`', async () => { | ||
| const storage = getFreshStorage() | ||
| const { context, persister, query, queryFn, storageKey } = setupPersister( | ||
| ['foo'], | ||
| { | ||
| storage, | ||
| }, | ||
| ) | ||
|
|
||
| await storage.setItem( | ||
| storageKey, | ||
| JSON.stringify({ | ||
| buster: '', | ||
| state: { dataUpdatedAt: Date.now() + 1000, data: '' }, | ||
| }), | ||
| ) | ||
|
|
||
| await persister.persisterFn(queryFn, context, query) | ||
| query.state.isInvalidated = true | ||
| query.fetch = vi.fn() | ||
|
|
||
| await vi.advanceTimersByTimeAsync(0) | ||
|
|
||
| expect(queryFn).toHaveBeenCalledTimes(0) | ||
| expect(query.fetch).toHaveBeenCalledTimes(1) | ||
| }) |
There was a problem hiding this comment.
Tests are not passing the refetchOnRestore option.
The test claims to verify behavior when refetchOnRestore is set to 'always', but the persister is created without passing this option. The test should include refetchOnRestore: 'always' in the options object passed to setupPersister:
Apply this diff to fix the test:
test('should restore item from the storage and refetch when `refetchOnRestore` is set to `always`', async () => {
const storage = getFreshStorage()
const { context, persister, query, queryFn, storageKey } = setupPersister(
['foo'],
{
storage,
+ refetchOnRestore: 'always',
},
)Additionally, to properly test that 'always' refetches even when data is NOT stale, consider removing the query.state.isInvalidated = true line (Line 281), since the future dataUpdatedAt already ensures the query appears fresh, and 'always' should refetch regardless of staleness.
📝 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.
| test('should restore item from the storage and refetch when `refetchOnRestore` is set to `always`', async () => { | |
| const storage = getFreshStorage() | |
| const { context, persister, query, queryFn, storageKey } = setupPersister( | |
| ['foo'], | |
| { | |
| storage, | |
| }, | |
| ) | |
| await storage.setItem( | |
| storageKey, | |
| JSON.stringify({ | |
| buster: '', | |
| state: { dataUpdatedAt: Date.now() + 1000, data: '' }, | |
| }), | |
| ) | |
| await persister.persisterFn(queryFn, context, query) | |
| query.state.isInvalidated = true | |
| query.fetch = vi.fn() | |
| await vi.advanceTimersByTimeAsync(0) | |
| expect(queryFn).toHaveBeenCalledTimes(0) | |
| expect(query.fetch).toHaveBeenCalledTimes(1) | |
| }) | |
| test('should restore item from the storage and refetch when `refetchOnRestore` is set to `always`', async () => { | |
| const storage = getFreshStorage() | |
| const { context, persister, query, queryFn, storageKey } = setupPersister( | |
| ['foo'], | |
| { | |
| storage, | |
| refetchOnRestore: 'always', | |
| }, | |
| ) | |
| await storage.setItem( | |
| storageKey, | |
| JSON.stringify({ | |
| buster: '', | |
| state: { dataUpdatedAt: Date.now() + 1000, data: '' }, | |
| }), | |
| ) | |
| await persister.persisterFn(queryFn, context, query) | |
| query.state.isInvalidated = true | |
| query.fetch = vi.fn() | |
| await vi.advanceTimersByTimeAsync(0) | |
| expect(queryFn).toHaveBeenCalledTimes(0) | |
| expect(query.fetch).toHaveBeenCalledTimes(1) | |
| }) |
🤖 Prompt for AI Agents
In packages/query-persist-client-core/src/__tests__/createPersister.test.ts
around lines 263 to 288, the test claims to verify refetchOnRestore: 'always'
but does not pass that option and also manually marks the query as invalidated;
update the setupPersister call to include refetchOnRestore: 'always' in the
options object and remove the line that sets query.state.isInvalidated = true so
the test verifies that 'always' triggers a refetch even when the restored data
is fresh.
packages/query-persist-client-core/src/__tests__/createPersister.test.ts
Show resolved
Hide resolved
DamianOsipiuk
force-pushed
the
do/persister-add-refetch-control
branch
from
e6ce6e6 to
a462859
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9745 +/- ##
===========================================
+ Coverage 45.51% 80.88% +35.36%
===========================================
Files 196 36 -160
Lines 8324 659 -7665
Branches 1902 186 -1716
===========================================
- Hits 3789 533 -3256
+ Misses 4093 109 -3984
+ Partials 442 17 -425
🚀 New features to boost your workflow:
|
…tch after restore (TanStack#9745)
1 participant
🎯 Changes
Fixes: #9667, #9683
✅ Checklist
pnpm test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Documentation
Tests
Chores