-
Notifications
You must be signed in to change notification settings - Fork 0
feat(docs): add the angular testing documentation #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new Angular unit-testing example (app, tests, configs), a new Angular testing guide, and documentation nav entries. Also tweaks example services/mutations (invalidateQueries return values and optimistic-update handlers) and updates an interceptor error path and a devtools-panel tsconfig flag. Changes
Sequence Diagram(s)sequenceDiagram
participant C as UnitTestingComponent
participant S as TasksService
participant QC as QueryClient
participant HTTP as HttpClient
participant INT as MockInterceptor
C->>QC: injectQuery(allTasks)
QC->>S: call queryFn()
S->>HTTP: GET /api/tasks
HTTP->>INT: intercept GET
INT-->>HTTP: respond 200 [] (delayed)
HTTP-->>S: return []
S-->>QC: fulfill query
QC-->>C: data=[]
C->>QC: injectMutation(addTask).mutate({task, fail?})
QC->>S: call mutationFn()
S->>QC: onMutate (cancel + optimistic update)
S->>HTTP: POST /api/tasks or /api/tasks-wrong-url
HTTP->>INT: intercept POST
alt success
INT-->>HTTP: 201 success (delayed)
HTTP-->>S: success
S->>QC: onSuccess (no-op)
S->>QC: onSettled -> return invalidateQueries()
else error
INT-->>HTTP: throw Error (delayed)
HTTP-->>S: error
S->>QC: onError -> rollback using context
S->>QC: onSettled -> return invalidateQueries()
end
QC->>S: refetch allTasks
sequenceDiagram
participant TB as TestBed
participant INS as Injector (runInInjectionContext)
participant S as TasksService
participant QC as QueryClient
participant INT as MockInterceptor
TB->>INS: provideHttpClient(withInterceptors([mockInterceptor]))
TB->>INS: provideTanStackQuery(new QueryClient({ defaultOptions... , retries:0 }))
INS->>S: inject TasksService
INS->>QC: injectQuery(allTasks) / injectMutation(addTask)
QC->>S: execute queryFn / mutationFn
S->>INT: HTTP interactions intercepted & delayed
TB->>QC: poll isFetching / inspect getQueryData for optimistic behavior and rollbacks
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @arvi18, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've focused on significantly improving the Angular documentation and examples related to testing with TanStack Query. This pull request introduces a dedicated guide for testing best practices and a fully functional unit testing example project. Additionally, I've made minor but important adjustments to existing examples to enhance error simulation and refine mutation lifecycle handling, ensuring a more robust and type-safe experience for developers.
Highlights
- New Angular Testing Documentation: I've added a brand new documentation guide at
docs/framework/angular/guides/testing.md
. This guide provides comprehensive information on how to effectively test Angular applications that utilize TanStack Query, including strategies for handling asynchronous operations like polling with@testing-library/angular
and managing query retries in tests. - New Angular Unit Testing Example Project: A complete new example project,
examples/angular/unit-testing/
, has been introduced. This project serves as a practical demonstration of setting up and performing unit tests for Angular applications with TanStack Query, featuring Jest and ESLint configurations, a sample service, a component, and a mock API interceptor for realistic testing scenarios. - Improved Error Simulation in Mock API Interceptors: The mock API interceptors in both the
optimistic-updates
example and the newunit-testing
example have been refined. They now usethrowError
from RxJS for simulating network errors, which provides a more accurate and robust way to test error handling compared to simply returning a 500 status response. - Enhanced Mutation and Query Lifecycle Handling: I've enhanced the mutation and query lifecycle handling in the
examples/angular/auto-refetching/src/app/services/tasks.service.ts
andexamples/angular/optimistic-updates/src/app/services/tasks.service.ts
files. This includes explicitly returninginvalidateQueries
calls inonSuccess
andonSettled
callbacks and improving type safety foronMutate
andonError
callback parameters, leading to more predictable and robust state management. - Documentation Navigation Updates: The
docs/config.json
file has been updated to seamlessly integrate the new content into the documentation navigation. This includes adding direct links to the new 'Testing' guide and the 'Unit Testing / Jest' example, making them easily discoverable for users.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
🤖 Refacto.AI: PR Summary & Reviewer Guide🎯 1. The Goal (Why was this PR raised?)This PR adds comprehensive documentation and examples for testing Angular applications that use TanStack Query. It introduces both testing guides and a complete working example demonstrating best practices for unit testing Angular services with TanStack Query. 🗺️ 2. The Changes (What was implemented?)
🤔 3. Key Areas for Human ReviewArea of Concern: Testing Documentation Completeness
Area of Concern: Test Example Implementation
Area of Concern: Mock API Interceptor
Area of Concern: Consistency with Other Frameworks
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces comprehensive documentation and a new example project for testing Angular applications with TanStack Query. The changes are extensive and well-structured. My review focuses on improving the new documentation and example code by enhancing type safety and addressing potential issues like stateful mocks that could lead to flaky tests. I've provided specific suggestions to use proper TypeScript types instead of any
, fix an error in a code example, and improve the robustness of the test setup.
injector = TestBed.inject(Injector); | ||
}); | ||
|
||
it('should get all the Tasks', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test function it('should get all the Tasks', ...)
uses await
on line 99, but it is not declared as async
. This will result in a syntax error and the test will not run as expected. Test functions that use await
must be declared async
.
it('should get all the Tasks', () => { | |
it('should get all the Tasks', async () => { |
}); | ||
|
||
it('should get all the Tasks', () => { | ||
let allTasks: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The allTasks
variable is typed as any
. Using any
defeats the purpose of TypeScript and can hide potential bugs. Since CreateQueryResult
is already imported, you should use it to properly type the variable. This will improve type safety and provide better autocompletion.
let allTasks: any; | |
let allTasks: CreateQueryResult<string[], Error>; |
} from '@angular/common/http' | ||
import type { Observable } from 'rxjs' | ||
|
||
let callNumber = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The callNumber
variable is declared at the module level, which makes it stateful across multiple tests if they run in the same process without module cache invalidation. This can lead to flaky and unpredictable test outcomes. It's crucial to reset this state before each test. Consider exporting a reset function from this module to be called in a beforeEach
block in your spec files.
Example:
// In mock-api.interceptor.ts
export function resetCallNumber() {
callNumber = 0;
}
// In your spec file
import { resetCallNumber } from '...';
describe('...', () => {
beforeEach(() => {
resetCallNumber();
// ... other setup
});
});
let service!: TasksService; | ||
let injector!: Injector; | ||
let allTasks: CreateQueryResult<Array<string>, Error>; | ||
let addTask: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The addTask
variable is typed as any
. This undermines TypeScript's type safety. You should provide a proper type by importing CreateMutationResult
from @tanstack/angular-query-experimental
and using it here. This will make your tests more robust and easier to maintain.
You'll need to add CreateMutationResult
to your import on line 8.
let addTask: any; | |
let addTask: CreateMutationResult<any, Error, { task: string; failMutation: boolean; }>; |
onSuccess: (data: any, variables: any, _context: any) => { | ||
expect(data).toEqual({ | ||
status: 'success', | ||
task: task | ||
}); | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameters data
and variables
in the onSuccess
callback are implicitly typed as any
. Using explicit types improves code clarity and safety. Based on the mutation, data
is the response from the POST request and variables
is what's passed to mutateAsync
.
onSuccess: (data: any, variables: any, _context: any) => { | |
expect(data).toEqual({ | |
status: 'success', | |
task: task | |
}); | |
}, | |
onSuccess: (data: { status: string; task: string; }, _variables: { task: string; failMutation: boolean; }, _context: unknown) => { | |
expect(data).toEqual({ | |
status: 'success', | |
task: task | |
}); | |
}, |
onError: (data: any, _variables: any, _context: any) => { | ||
expect(data).toEqual(new Error('error')); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The data
parameter in the onError
callback is implicitly typed as any
. For better type safety, it should be typed as Error
. The other parameters should also be typed.
onError: (data: any, _variables: any, _context: any) => { | |
expect(data).toEqual(new Error('error')); | |
} | |
onError: (data: Error, _variables: { task: string; failMutation: boolean; }, _context: unknown) => { | |
expect(data).toEqual(new Error('error')); | |
} |
queryOptions({ | ||
queryKey: ['tasks'], | ||
queryFn: () => { | ||
return lastValueFrom(this.#http.get<Array<string>>('/api/tasks')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// We await the first result from the query | ||
await waitFor(() => expect(allTasks.isFetching()).toBe(false), {timeout: 10000}); | ||
expect(allTasks.status()).toEqual('success'); | ||
expect(allTasks.data()).toEqual([]); // Considering that the inteceptor is returning [] at the first query request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the comment. "inteceptor" should be "interceptor".
expect(allTasks.data()).toEqual([]); // Considering that the inteceptor is returning [] at the first query request. | |
expect(allTasks.data()).toEqual([]); // Considering that the interceptor is returning [] at the first query request. |
), | ||
mutationKey: ['tasks'], | ||
onSuccess: () => {}, | ||
onMutate: async ({ task }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
return previousTodos | ||
}, | ||
onError: (_err, _variables, context) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/refacto-test |
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Code Review: Angular Testing with TanStack Query👍 Well Done
📌 Files Processed
📝 Additional Comments
|
allTasks = () => | ||
queryOptions({ | ||
queryKey: ['tasks'], | ||
queryFn: () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing Error Handling
HTTP request lacks error handling, causing unhandled promise rejections. Network failures or server errors would propagate uncaught, potentially crashing the application.
queryFn: () => { | |
import { catchError } from 'rxjs/operators'; | |
import { of } from 'rxjs'; | |
// In the queryFn: | |
return lastValueFrom(this.#http.get<Array<string>>('/api/tasks').pipe( | |
catchError(error => { | |
console.error('Failed to fetch tasks:', error); | |
return of([]); // Return empty array as fallback | |
}) | |
)); |
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- SRE-Error-Handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
invalid
this.addMutation.mutate({ | ||
task: this.newItem, | ||
failMutation: this.failMutation, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unhandled Promise Rejection
Mutation is called without error handling. Failures in the mutation would be unhandled, potentially causing UI inconsistency and silent failures.
this.addMutation.mutate({ | |
task: this.newItem, | |
failMutation: this.failMutation, | |
}) | |
this.addMutation.mutate( | |
{ | |
task: this.newItem, | |
failMutation: this.failMutation, | |
}, | |
{ | |
onError: (error) => { | |
console.error('Failed to add task:', error); | |
// Optional: Add UI feedback for the user | |
} | |
} | |
) |
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- SRE-Error-Handling
onSettled: () => { | ||
return this.#queryClient.invalidateQueries({ queryKey: ['tasks'] }) | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inefficient Query Invalidation
Query invalidation occurs on every mutation regardless of success/error state. This causes unnecessary refetching when errors occur, wasting network resources and processing time.
onSettled: () => { | |
return this.#queryClient.invalidateQueries({ queryKey: ['tasks'] }) | |
}, | |
onSuccess: () => { | |
return this.#queryClient.invalidateQueries({ queryKey: ['tasks'] }) | |
}, | |
onError: (_err, _variables, context) => { | |
if (context) { | |
// Rollback the optimistic update | |
this.#queryClient.setQueryData(['tasks'], context) | |
} | |
} |
Standards
- ISO-IEC-25010-Performance-Resource-Utilization
- Algorithm-Opt-Conditional-Execution
}) | ||
} | ||
} | ||
if (req.url === '/api/tasks-wrong-url') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error Disclosure Risk
Generic error message directly exposed in API response. Attackers could use detailed error information for reconnaissance. Sensitive implementation details could be leaked.
if (req.url === '/api/tasks-wrong-url') { | |
return throwError(() => new Error('Invalid request')).pipe(delay(1000)); |
Standards
- CWE-209
- OWASP-A07
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code suggestion on invalid lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (23)
examples/angular/unit-testing/jest.config.ts (1)
1-1
: Fix TypeScript import for better type safety.The ESLint error indicates that
import()
type annotations should be avoided in favor of explicit imports.Apply this diff to use a proper import:
+import type { Config } from 'jest'; + -const config: import('jest').Config = { +const config: Config = {examples/angular/unit-testing/tsconfig.spec.json (1)
3-7
: Consider adding Node type definitions if tests reference Node globalsIf your Jest setup or helpers reference Node globals (e.g., process, __dirname, setImmediate), add node to the types array. Otherwise, this is fine as-is.
Example:
- "types": ["jest"] + "types": ["jest", "node"]examples/angular/unit-testing/.eslintrc.cjs (1)
3-6
: Empty ESLint config — consider extending recommended presets for Angular, TypeScript, and JestIf this example is meant to be self-contained, extend standard presets. If you rely on a repo-level config, ignore this.
Proposed minimal config (requires corresponding deps installed):
-/** @type {import('eslint').Linter.Config} */ -const config = {} - -module.exports = config +/** @type {import('eslint').Linter.Config} */ +const config = { + root: true, + env: { es2021: true, browser: true, jest: true }, + extends: [ + 'eslint:recommended', + 'plugin:@typescript-eslint/recommended', + 'plugin:@angular-eslint/recommended', + 'plugin:jest/recommended', + ], + parser: '@typescript-eslint/parser', + parserOptions: { + sourceType: 'module', + project: ['./tsconfig.json', './tsconfig.spec.json'], + tsconfigRootDir: __dirname, + }, + overrides: [ + { + files: ['**/*.spec.ts'], + env: { jest: true }, + }, + ], +} + +module.exports = configexamples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts (2)
18-19
: Stateful callNumber may cause cross-test couplingcallNumber persists across GETs and tests. Ensure tests reset it (or sessionStorage) between runs to avoid order-dependent assertions.
Options:
- Reset between tests (e.g., in beforeEach): callNumber = 0; sessionStorage.removeItem('unit-testing-tasks')
- Or derive the suffix purely from storage length to keep it stateless.
28-43
: Minor readability: de-duplicate JSON.parse and simplify GET caseNot required, but factoring tasks into a local constant avoids repeating JSON.parse and clarifies the first vs subsequent call paths.
Example:
- case 'GET': - callNumber++; - if (callNumber === 1) { - return respondWith( - 200, - JSON.parse( - sessionStorage.getItem('unit-testing-tasks') || '[]', - ), - ) } else { - return respondWith( - 200, - JSON.parse( - sessionStorage.getItem('unit-testing-tasks') || '[]', - ).concat([`CallNumber ${callNumber}`]), - ) - } + case 'GET': { + callNumber++ + const tasks: string[] = JSON.parse( + sessionStorage.getItem('unit-testing-tasks') || '[]', + ) + return callNumber === 1 + ? respondWith(200, tasks) + : respondWith(200, tasks.concat([`CallNumber ${callNumber}`])) + }examples/angular/optimistic-updates/src/app/services/tasks.service.ts (2)
50-50
: Remove no-op onSuccess for claritySince onSettled handles invalidation, this empty handler adds noise.
- onSuccess: () => {},
51-75
: Align onMutate/variables typing to the mutation input for clarityonMutate narrows variables to { task: string } while mutationFn accepts { task: string; failMutation: boolean }. It works, but aligning types improves readability and future refactors.
Example:
- onMutate: async ({ task } : {task: string}) => { + onMutate: async ({ task }: { task: string; failMutation: boolean }) => { // Cancel any outgoing refetchesOptionally define a shared type:
type AddTaskVariables = { task: string; failMutation: boolean }…and use it in mutationFn, onMutate, and onError.
examples/angular/unit-testing/src/app/services/tasks.service.ts (3)
15-16
: Use readonly private fields for consistency and intentMatches other examples and prevents accidental reassignment.
- #queryClient = inject(QueryClient) // Manages query state and caching - #http = inject(HttpClient) // Handles HTTP requests + readonly #queryClient = inject(QueryClient) // Manages query state and caching + readonly #http = inject(HttpClient) // Handles HTTP requests
31-33
: Update the comment to reflect actual invalidation behaviorInvalidation happens in onSettled, while onSuccess is a no-op here.
- * On success, invalidates and refetch the "tasks" query cache to update the task list. + * Uses optimistic updates; always invalidates and refetches the "tasks" query in onSettled.
51-55
: Add an explicit type to onMutate parameter for clarityIt’s inferred, but explicit typing helps readability and prevents regressions.
- onMutate: async ({ task }) => { + onMutate: async ({ task }: { task: string; failMutation: boolean }) => {Optional: define a shared type and reuse across mutationFn, onMutate, and onError:
type AddTaskVariables = { task: string; failMutation: boolean }examples/angular/unit-testing/src/app/services/tasks.service.spec.ts (6)
1-1
: Use single quotes for consistencyAngular style guide recommends single quotes for imports.
-import { TestBed } from "@angular/core/testing"; +import { TestBed } from '@angular/core/testing';
2-2
: Use single quotes for consistency-import { provideHttpClient, withFetch, withInterceptors } from "@angular/common/http"; +import { provideHttpClient, withFetch, withInterceptors } from '@angular/common/http';
3-3
: Use single quotes for consistency-import { QueryClient, injectMutation, injectQuery, provideTanStackQuery } from "@tanstack/angular-query-experimental"; +import { QueryClient, injectMutation, injectQuery, provideTanStackQuery } from '@tanstack/angular-query-experimental';
4-4
: Use single quotes for consistency-import { Injector, inject, runInInjectionContext } from "@angular/core"; +import { Injector, inject, runInInjectionContext } from '@angular/core';
6-8
: Use single quotes for consistency-import { mockInterceptor } from "../interceptor/mock-api.interceptor"; -import { TasksService } from "./tasks.service"; -import type { CreateQueryResult} from "@tanstack/angular-query-experimental"; +import { mockInterceptor } from '../interceptor/mock-api.interceptor'; +import { TasksService } from './tasks.service'; +import type { CreateQueryResult, CreateMutationResult } from '@tanstack/angular-query-experimental';
110-110
: Add missing semicolon- expect(allTasks.data()).toEqual([task, 'CallNumber 2']) + expect(allTasks.data()).toEqual([task, 'CallNumber 2']);examples/angular/unit-testing/angular.json (2)
81-81
: Default build to development for examples (faster DX)For a docs/example project, using development as the default build configuration improves iteration speed and keeps source maps on. Consider switching the default or aligning serve/build defaults.
- "defaultConfiguration": "production" + "defaultConfiguration": "development"
15-41
: Reconsider global skipTests in a testing-focused exampleGiven this project teaches testing, you might want the schematics to generate test files by default (at least for services/components) to reduce friction for learners.
- "@schematics/angular:component": { - "inlineTemplate": true, - "inlineStyle": true, - "skipTests": true - }, + "@schematics/angular:component": { + "inlineTemplate": true, + "inlineStyle": true, + "skipTests": false + }, - "@schematics/angular:service": { - "skipTests": true - } + "@schematics/angular:service": { + "skipTests": false + }docs/framework/angular/guides/testing.md (5)
6-15
: Clarify guidance, fix grammar, and reference toObservable correctlyTighten phrasing, correct grammar, and add the import location for toObservable.
-As there is currently no simple way to await a signal to reach a specific value we will use polling to wait in our test (instead of transforming our signals in observable and use RxJS features to filter the values). If you want to do like us for the polling you can use the angular testing library. +As there is currently no simple way to await a signal to reach a specific value, we’ll use polling in tests (instead of transforming signals into observables and filtering with RxJS). If you prefer polling, you can use the Angular Testing Library. @@ -Otherwise we recommend to use the toObservable feature from Angular. +Otherwise, we recommend using Angular’s toObservable utility: + +import { toObservable } from '@angular/core/rxjs-interop'
41-51
: Return type comment mismatch: this returns a Promise, not an ObservableThe code uses lastValueFrom and returns a Promise. Adjust the comment to avoid confusion.
- /** - * Fetches all tasks from the API. - * Returns an observable containing an array of task strings. - */ + /** + * Fetches all tasks from the API. + * Returns a Promise that resolves to an array of task strings. + */
56-65
: Optional: strengthen types for the query resultSince you already import CreateQueryResult, you can type the variable for better IntelliSense and safety.
-import type { CreateQueryResult} from "@tanstack/angular-query-experimental"; +import type { CreateQueryResult } from "@tanstack/angular-query-experimental"; @@ - let allTasks: any; + let allTasks: CreateQueryResult<Array<string>>;Also applies to: 91-95
126-133
: Speed up tests by reducing artificial delays or using fake timersThe interceptor adds 1000ms delays to every call, which slows the suite. Consider reducing to 50–100ms, or use Jest fake timers and advanceTimersByTime in tests.
- const respondWith = (status: number, body: any) => - of(new HttpResponse({ status, body })).pipe(delay(1000)) + const respondWith = (status: number, body: any) => + of(new HttpResponse({ status, body })).pipe(delay(100)) @@ - if (req.url === '/api/tasks-wrong-url') { - return throwError(() => new Error('error')).pipe(delay(1000)); + if (req.url === '/api/tasks-wrong-url') { + return throwError(() => new Error('error')).pipe(delay(100));Also applies to: 156-161
170-171
: Fix typos and pluralizationMinor editorial fixes for clarity.
-Instead of targetting a server for the data you should mock the requests. There are multiple way of handling the mocking, we recommend to use the Interceptor from Angular, see [here](https://angular.dev/guide/http/interceptors) for more details. -You can see the the Interceptor setup in the "Unit testing / Jest" examples. +Instead of targeting a server for data, you should mock requests. There are multiple ways to handle mocking; we recommend using an Angular HTTP interceptor. See [the docs](https://angular.dev/guide/http/interceptors) for details. +You can see the interceptor setup in the “Unit testing / Jest” examples.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
examples/angular/unit-testing/src/favicon.ico
is excluded by!**/*.ico
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
docs/config.json
(2 hunks)docs/framework/angular/guides/testing.md
(1 hunks)examples/angular/auto-refetching/src/app/services/tasks.service.ts
(2 hunks)examples/angular/optimistic-updates/src/app/components/optimistic-updates.component.ts
(0 hunks)examples/angular/optimistic-updates/src/app/interceptor/mock-api.interceptor.ts
(2 hunks)examples/angular/optimistic-updates/src/app/services/tasks.service.ts
(2 hunks)examples/angular/unit-testing/.devcontainer/devcontainer.json
(1 hunks)examples/angular/unit-testing/.eslintrc.cjs
(1 hunks)examples/angular/unit-testing/README.md
(1 hunks)examples/angular/unit-testing/angular.json
(1 hunks)examples/angular/unit-testing/jest.config.ts
(1 hunks)examples/angular/unit-testing/package.json
(1 hunks)examples/angular/unit-testing/src/app/app.component.ts
(1 hunks)examples/angular/unit-testing/src/app/app.config.ts
(1 hunks)examples/angular/unit-testing/src/app/components/unit-testing.component.ts
(1 hunks)examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts
(1 hunks)examples/angular/unit-testing/src/app/services/tasks.service.spec.ts
(1 hunks)examples/angular/unit-testing/src/app/services/tasks.service.ts
(1 hunks)examples/angular/unit-testing/src/index.html
(1 hunks)examples/angular/unit-testing/src/main.ts
(1 hunks)examples/angular/unit-testing/tsconfig.app.json
(1 hunks)examples/angular/unit-testing/tsconfig.json
(1 hunks)examples/angular/unit-testing/tsconfig.spec.json
(1 hunks)
💤 Files with no reviewable changes (1)
- examples/angular/optimistic-updates/src/app/components/optimistic-updates.component.ts
🧰 Additional context used
🧬 Code Graph Analysis (7)
examples/angular/unit-testing/src/app/app.component.ts (2)
examples/angular/optimistic-updates/src/app/components/optimistic-updates.component.ts (1)
Component
(10-72)examples/angular/unit-testing/src/app/components/unit-testing.component.ts (1)
Component
(10-67)
examples/angular/unit-testing/src/main.ts (1)
examples/angular/unit-testing/src/app/app.config.ts (1)
appConfig
(14-28)
examples/angular/unit-testing/src/app/app.config.ts (2)
examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts (1)
mockInterceptor
(20-64)packages/angular-query-experimental/src/providers.ts (2)
provideTanStackQuery
(93-111)withDevtools
(242-335)
examples/angular/unit-testing/src/app/components/unit-testing.component.ts (4)
examples/angular/optimistic-updates/src/app/components/optimistic-updates.component.ts (1)
Component
(10-72)examples/angular/unit-testing/src/app/app.component.ts (1)
Component
(4-11)packages/angular-query-experimental/src/inject-query.ts (1)
injectQuery
(218-225)packages/angular-query-experimental/src/inject-mutation.ts (1)
injectMutation
(41-174)
examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts (1)
examples/angular/optimistic-updates/src/app/interceptor/mock-api.interceptor.ts (1)
mockInterceptor
(18-53)
examples/angular/unit-testing/src/app/services/tasks.service.spec.ts (7)
examples/angular/optimistic-updates/src/app/services/tasks.service.ts (1)
addTask
(34-81)examples/angular/unit-testing/src/app/services/tasks.service.ts (1)
addTask
(34-82)examples/angular/optimistic-updates/src/app/interceptor/mock-api.interceptor.ts (1)
mockInterceptor
(18-53)examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts (1)
mockInterceptor
(20-64)packages/angular-query-experimental/src/providers.ts (1)
provideTanStackQuery
(93-111)packages/angular-query-experimental/src/inject-query.ts (1)
injectQuery
(218-225)packages/angular-query-experimental/src/inject-mutation.ts (1)
injectMutation
(41-174)
examples/angular/unit-testing/src/app/services/tasks.service.ts (2)
examples/angular/auto-refetching/src/app/services/tasks.service.ts (1)
Injectable
(11-59)examples/angular/optimistic-updates/src/app/services/tasks.service.ts (1)
Injectable
(11-82)
🪛 ESLint
examples/angular/unit-testing/jest.config.ts
[error] 1-1: import()
type annotations are forbidden.
(@typescript-eslint/consistent-type-imports)
🪛 Biome (2.1.2)
examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts
[error] 45-47: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (19)
examples/angular/unit-testing/.devcontainer/devcontainer.json (1)
1-4
: LGTM!The devcontainer configuration is clean and appropriate for Node.js 22 development environment.
examples/angular/unit-testing/README.md (1)
1-7
: LGTM!The README provides clear and comprehensive instructions covering all major package managers and essential commands.
examples/angular/unit-testing/src/index.html (1)
1-13
: LGTM!Standard and well-structured HTML5 boilerplate for Angular application with appropriate meta tags and bootstrap element.
examples/angular/unit-testing/jest.config.ts (1)
1-5
: Jest configuration is functional and appropriate.The configuration correctly sets up Jest for the Angular project with proper source directory roots.
examples/angular/auto-refetching/src/app/services/tasks.service.ts (2)
40-42
: Improved async handling in mutation callback.Returning the promise from
invalidateQueries
provides better control flow and allows callers to wait for cache invalidation completion.
54-56
: Consistent async handling across mutations.Good consistency with the addTask mutation - both now return promises from their onSuccess callbacks.
examples/angular/optimistic-updates/src/app/interceptor/mock-api.interceptor.ts (2)
9-9
: Good addition of throwError for improved error simulation.Adding throwError import enables more realistic error scenarios for testing.
49-49
: Enhanced error simulation for better testing.Using
throwError
instead of a 500 HttpResponse provides more realistic error scenarios that better simulate actual network failures.examples/angular/unit-testing/package.json (1)
1-37
: Comprehensive and well-structured package configuration.The dependencies are appropriate for Angular 19 with Jest testing setup. Good inclusion of Testing Library packages and consistent version pinning.
examples/angular/unit-testing/tsconfig.json (1)
1-32
: Excellent TypeScript configuration with comprehensive strict settings.The configuration enables all recommended strict type checking options and Angular compiler strictness settings, promoting type safety and best practices.
examples/angular/optimistic-updates/src/app/services/tasks.service.ts (1)
77-79
: LGTM: returning invalidateQueries from onSettledReturning the promise is useful for test determinism and consistent async handling.
examples/angular/unit-testing/src/app/services/tasks.service.ts (1)
77-79
: LGTM: returning invalidateQueries from onSettledGood for deterministic tests and consistent async flows.
examples/angular/unit-testing/tsconfig.app.json (1)
1-9
: LGTM!Standard Angular TypeScript configuration setup. The empty
types
array correctly excludes test types from the app build.examples/angular/unit-testing/src/main.ts (1)
1-5
: LGTM!Standard Angular standalone application bootstrap with proper error handling.
examples/angular/unit-testing/src/app/app.component.ts (1)
1-11
: LGTM!Clean root component setup with OnPush change detection strategy and proper standalone configuration.
examples/angular/unit-testing/src/app/app.config.ts (1)
1-28
: LGTM!Well-configured application setup with HTTP client, mock interceptor, and TanStack Query. The 24-hour gcTime is appropriate for the testing example.
docs/config.json (1)
710-713
: LGTM!Documentation navigation entries correctly added for Angular testing guide and unit testing example.
Also applies to: 1291-1294
examples/angular/unit-testing/src/app/components/unit-testing.component.ts (1)
1-67
: LGTM!Well-structured component demonstrating TanStack Query integration with Angular. Proper use of OnPush change detection, new control flow syntax, and reactive state management.
examples/angular/unit-testing/src/app/services/tasks.service.spec.ts (1)
46-111
: Well-structured and comprehensive test coverage!The test suite effectively covers:
- Initial query state validation
- Successful mutations with optimistic updates
- Error handling with rollback behavior
- Query cache inspection and validation
- Proper async testing patterns with
waitFor
Great job on providing thorough test examples for the Angular TanStack Query integration.
it('should get all the Tasks', () => { | ||
let allTasks: any; | ||
runInInjectionContext(injector, () => { | ||
allTasks = injectQuery(() => service.allTasks()); | ||
}); | ||
expect(allTasks.status()).toEqual('pending'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make the test async to use await
The test uses await but the it callback isn’t async, which will cause a failure. Also fix a typo in the trailing comment.
-it('should get all the Tasks', () => {
+it('should get all the Tasks', async () => {
@@
- expect(allTasks.data()).toEqual([]); // Considering that the inteceptor is returning [] at the first query request.
+ expect(allTasks.data()).toEqual([]); // Considering that the interceptor returns [] on the first request.
Also applies to: 99-103
🤖 Prompt for AI Agents
In docs/framework/angular/guides/testing.md around lines 90-95 (and also apply
the same change to lines 99-103): the test callback is missing async while it
uses await and there’s a typo in the trailing comment; make the it(...) callback
async, await the call that returns the task result (e.g., await injectQuery(...)
or await runInInjectionContext(...) as appropriate), and correct the trailing
comment typo so the test executes as intended and the comment reads correctly.
"test": { | ||
"builder": "@angular-builders/jest:run", | ||
"options": { | ||
"tsConfig": "tsconfig.spec.json", | ||
"configPath": "jest.config.ts", | ||
"rootDir": "." | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify Jest builder/config presence and dependencies
Ensure the referenced Jest setup exists and required packages are installed to avoid CI/runtime failures.
Run this script to validate presence and dependencies:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
BASE="examples/angular/unit-testing"
# Check files referenced by the 'test' target
for f in "$BASE/jest.config.ts" "$BASE/tsconfig.spec.json" "$BASE/angular.json"; do
if [[ ! -f "$f" ]]; then
echo "Missing file: $f" && exit 1
fi
done
# Check package.json for jest builder and preset
PKG="$BASE/package.json"
if [[ -f "$PKG" ]]; then
echo "Checking devDependencies in $PKG"
jq -e '.devDependencies["@angular-builders/jest"]' "$PKG" >/dev/null || { echo "Missing @angular-builders/jest"; exit 1; }
jq -e '.devDependencies["jest"]' "$PKG" >/dev/null || { echo "Missing jest"; exit 1; }
jq -e '.devDependencies["jest-preset-angular"]' "$PKG" >/dev/null || { echo "Missing jest-preset-angular"; exit 1; }
else
echo "Missing $PKG" && exit 1
fi
# Check zone.js is installed (polyfills includes it)
jq -e '.dependencies["zone.js"] // .devDependencies["zone.js"]' "$PKG" >/dev/null || { echo "Missing zone.js dependency"; exit 1; }
echo "Jest setup looks present."
Length of output: 1073
🏁 Script executed:
#!/bin/bash
set -euo pipefail
PKG="examples/angular/unit-testing/package.json"
echo "Zone.js in dependencies: $(jq -r '.dependencies["zone.js"] // "none"' $PKG)"
echo "Zone.js in devDependencies: $(jq -r '.devDependencies["zone.js"] // "none"' $PKG)"
Length of output: 399
Missing jest-preset-angular
in devDependencies
Our verification shows that while @angular-builders/jest
, jest
, and zone.js
are present, the jest-preset-angular
package is not listed under devDependencies in examples/angular/unit-testing/package.json
. Without it, the Jest setup will fail at runtime.
Please update your package.json:
• File: examples/angular/unit-testing/package.json
"devDependencies": {
"@angular-builders/jest": "<your-version>",
"jest": "<your-version>",
+ "jest-preset-angular": "<compatible-version>"
}
Once added, re-run npm install
(or yarn
) and verify the tests execute successfully.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
examples/angular/unit-testing/package.json: add the missing jest-preset-angular
entry to devDependencies so Jest can run with the Angular preset; update
package.json by adding a compatible "jest-preset-angular" version to
devDependencies (choose a version that matches your Angular/Jest setup), save
the file, then run npm install or yarn and verify tests run successfully.
case 'POST': | ||
const tasks = JSON.parse( | ||
sessionStorage.getItem('unit-testing-tasks') || '[]', | ||
) | ||
tasks.push(req.body) | ||
sessionStorage.setItem( | ||
'unit-testing-tasks', | ||
JSON.stringify(tasks), | ||
) | ||
return respondWith(201, { | ||
status: 'success', | ||
task: req.body, | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix switch-case declaration scoping (noSwitchDeclarations)
Declaring const tasks directly in a switch case can leak across cases. Wrap the case body in a block.
- case 'POST':
- const tasks = JSON.parse(
- sessionStorage.getItem('unit-testing-tasks') || '[]',
- )
- tasks.push(req.body)
- sessionStorage.setItem(
- 'unit-testing-tasks',
- JSON.stringify(tasks),
- )
- return respondWith(201, {
- status: 'success',
- task: req.body,
- })
+ case 'POST': {
+ const tasks = JSON.parse(
+ sessionStorage.getItem('unit-testing-tasks') || '[]',
+ )
+ tasks.push(req.body)
+ sessionStorage.setItem(
+ 'unit-testing-tasks',
+ JSON.stringify(tasks),
+ )
+ return respondWith(201, {
+ status: 'success',
+ task: req.body,
+ })
+ }
🧰 Tools
🪛 Biome (2.1.2)
[error] 45-47: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts
around lines 44 to 57, the switch case for 'POST' declares const tasks directly
which can leak into other cases; wrap the entire case body in a block (add { ...
} after case 'POST':) and move the const/let declarations and sessionStorage
manipulations inside that block so the variables are scoped to the case.
if (req.url === '/api/tasks-wrong-url') { | ||
return throwError(() => new Error('error')).pipe(delay(1000)); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Simulate HTTP errors with HttpErrorResponse for better realism
Returning a plain Error loses HTTP context (status, statusText, url). Prefer HttpErrorResponse so HttpClient error flows mirror real backend failures.
- if (req.url === '/api/tasks-wrong-url') {
- return throwError(() => new Error('error')).pipe(delay(1000));
- }
+ if (req.url === '/api/tasks-wrong-url') {
+ return throwError(
+ () =>
+ new HttpErrorResponse({
+ status: 500,
+ statusText: 'Internal Server Error',
+ url: req.url,
+ error: { message: 'error' },
+ }),
+ ).pipe(delay(1000))
+ }
Additionally add this import at the top:
-import { HttpResponse } from '@angular/common/http'
+import { HttpResponse, HttpErrorResponse } from '@angular/common/http'
📝 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.
if (req.url === '/api/tasks-wrong-url') { | |
return throwError(() => new Error('error')).pipe(delay(1000)); | |
} | |
// At the top of mock-api.interceptor.ts | |
-import { HttpResponse } from '@angular/common/http'; | |
+import { HttpResponse, HttpErrorResponse } from '@angular/common/http'; | |
// …later, around line 59… | |
if (req.url === '/api/tasks-wrong-url') { | |
- return throwError(() => new Error('error')).pipe(delay(1000)); | |
+ return throwError( | |
+ () => | |
+ new HttpErrorResponse({ | |
+ status: 500, | |
+ statusText: 'Internal Server Error', | |
+ url: req.url, | |
+ error: { message: 'error' }, | |
+ }), | |
+ ).pipe(delay(1000)); | |
} |
🤖 Prompt for AI Agents
In examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts
around lines 59 to 61, the interceptor returns a plain Error for the simulated
HTTP failure which loses HTTP context; replace the throwError(() => new
Error('error')) with throwError(() => new HttpErrorResponse({ status: 500,
statusText: 'Server Error', url: req.url, error: 'error' })) and ensure you add
the import for HttpErrorResponse from '@angular/common/http' at the top of the
file so the simulated error includes status, statusText and url like a real
backend response.
let service!: TasksService; | ||
let injector!: Injector; | ||
let allTasks: CreateQueryResult<Array<string>, Error>; | ||
let addTask: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add proper typing for addTask variable
Using any
type reduces type safety. Consider using the proper mutation result type.
- let addTask: any;
+ let addTask: CreateMutationResult<any, Error, { task: string; failMutation: boolean }, any>;
📝 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.
let addTask: any; | |
let addTask: CreateMutationResult<any, Error, { task: string; failMutation: boolean }, any>; |
🤖 Prompt for AI Agents
In examples/angular/unit-testing/src/app/services/tasks.service.spec.ts around
line 14, the local test variable "addTask" is declared as "any" which reduces
type safety; replace it with the concrete type returned by the service/mutation
(e.g., the service method return type or the mutation response interface) by
importing the correct response/interface from the service or feature module, or
derive it with ReturnType<typeof TasksService.prototype.addTask>; if it's a spy,
declare it as jasmine.SpyObj<ReturnType<typeof TasksService.prototype.addTask>>
or the concrete Observable/Promise<TaskResponse> type so tests retain full type
checking.
/refacto-test |
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/angular/devtools-panel/tsconfig.json (1)
2-2
: IscompileOnSave
necessary here? It’s editor-specific and often a no-op with modern Angular toolchains.This flag is primarily honored by certain editors (e.g., Visual Studio). VS Code and CLI/vite watchers typically ignore it, so it may add confusion without effect. If this is intentional for a specific workflow, consider documenting it in the devtools-panel README. Otherwise, consider removing for clarity.
Optional cleanup:
- "compileOnSave": true,
New Features
Introduced a comprehensive guide on testing Angular applications with TanStack Query, including examples and best practices.
Added a new Angular unit testing example project with full configuration, sample service, component, and mock API interceptor.
Provided ready-to-use Jest and ESLint configurations for Angular unit testing.
Included detailed documentation and setup instructions for the new unit testing example.
Bug Fixes
Improved error simulation in mock API interceptors for more accurate testing scenarios.
Refactor
Enhanced mutation and query lifecycle handling in Angular service examples for better type safety and control flow.
Documentation
Updated Angular documentation to include new testing guides and unit testing examples.
Summary by CodeRabbit
New Features
Tests
Documentation
Refactor
Chores