Conversation
…hing when 'isRestoring' is true
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded a test to the Angular Query experimental suite that verifies Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 4b1d9dc
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/angular-query-experimental/src/__tests__/inject-queries.test.ts (1)
88-89: Optional: useasync/awaitfor side-effectfulqueryFnmocks for readability.Non-blocking, but this can make intent a bit clearer when call counts are part of the assertion logic.
♻️ Suggested refactor
- const queryFn1 = vi.fn().mockImplementation(() => sleep(10).then(() => 1)) - const queryFn2 = vi.fn().mockImplementation(() => sleep(10).then(() => 2)) + const queryFn1 = vi.fn().mockImplementation(async () => { + await sleep(10) + return 1 + }) + const queryFn2 = vi.fn().mockImplementation(async () => { + await sleep(10) + return 2 + })Based on learnings: 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.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/angular-query-experimental/src/__tests__/inject-queries.test.ts` around lines 88 - 89, Replace the .then() mock implementations with explicit async/await functions for readability: update the vi.fn().mockImplementation for queryFn1 and queryFn2 to use async () => { await sleep(10); return <value>; } so side-effectful behavior and call counts are clearer (refer to queryFn1, queryFn2, and sleep).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/angular-query-experimental/src/__tests__/inject-queries.test.ts`:
- Around line 88-89: Replace the .then() mock implementations with explicit
async/await functions for readability: update the vi.fn().mockImplementation for
queryFn1 and queryFn2 to use async () => { await sleep(10); return <value>; } so
side-effectful behavior and call counts are clearer (refer to queryFn1,
queryFn2, and sleep).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea124edd-037f-461c-97a2-dd047a20345c
📒 Files selected for processing (1)
packages/angular-query-experimental/src/__tests__/inject-queries.test.ts
…est in 'describe' block for consistency with 'inject-query.test.ts'
1 participant
🎯 Changes
Add a test verifying that
injectQueriesdoes not fetch during the restoring period whenisRestoringistrue, matching the existing pattern ininjectQuerytests.This covers the
isRestoring() ? () => undefined : ...branch ininject-queries.ts.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit