Skip to content

Conversation

ExFlo
Copy link

@ExFlo ExFlo commented Apr 15, 2025

This PR is to improve the testing doc of Angular package.
Let's discuss what is missing and what you would like to be added.
BR

Summary by CodeRabbit

  • Documentation

    • Added an Angular Testing guide and updated docs navigation with “Testing” and “Unit Testing / Jest”.
  • Examples

    • Added a full Angular Unit Testing example (Jest, devcontainer, configs, mock API, components, service).
    • Auto-refetching: return promises from cache-invalidation callbacks.
    • Optimistic Updates: refined mutation lifecycle (invalidate on settle) and changed error emission behavior.
  • Tests

    • Added comprehensive unit tests for the Angular TasksService covering success, optimistic updates, retries, and rollback on errors.

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Apr 15, 2025
@ExFlo ExFlo force-pushed the feat/docsAngularTesting branch from cb6cea3 to 51a3661 Compare April 16, 2025 14:10
@arnoud-dv arnoud-dv self-assigned this Sep 2, 2025
Copy link
Contributor

coderabbitai bot commented Sep 11, 2025

Walkthrough

Adds Angular testing docs and navigation; introduces a new Angular "unit-testing" example (Jest) with mock HTTP interceptor, TasksService (optimistic updates, rollback, invalidate on settle), demo component, and comprehensive unit tests. Also tweaks two existing examples' mutation/invalidation behavior and an interceptor error emission.

Changes

Cohort / File(s) Summary
Docs navigation
docs/config.json
Adds Angular docs entries: framework/angular/guides/testing and framework/angular/examples/unit-testing.
Angular testing guide
docs/framework/angular/guides/testing.md
New guide covering setup, mocking via interceptors, sample TasksService, testing patterns and a Jest example reference.
Auto-refetching example: service
examples/angular/auto-refetching/src/app/services/tasks.service.ts
addTask and clearAllTasks onSuccess handlers now return the invalidateQueries({ queryKey: ['tasks'] }) Promise.
Optimistic updates: component
examples/angular/optimistic-updates/src/app/components/optimistic-updates.component.ts
Removed clearMutation property; no other logic changes.
Optimistic updates: mock interceptor
examples/angular/optimistic-updates/src/app/interceptor/mock-api.interceptor.ts
For /api/tasks-wrong-url, replaced HTTP 500 response with an error observable via throwError(() => new Error('error')).pipe(delay(1000)).
Optimistic updates: service
examples/angular/optimistic-updates/src/app/services/tasks.service.ts
Mutation callbacks refined: typed onMutate params, unused params prefixed with _, onSuccess made a no-op, onSettled returns the invalidateQueries Promise.
Unit-testing example: project scaffold
examples/angular/unit-testing/...
(.devcontainer/devcontainer.json, .eslintrc.cjs, package.json, angular.json, jest.config.ts, tsconfig*.json, README.md)
New Angular 19 + Jest project scaffold, build/test config, tooling and README for unit-testing example.
Unit-testing example: app scaffold
examples/angular/unit-testing/src/index.html, src/main.ts, src/app/app.component.ts, src/app/app.config.ts
Bootstraps standalone app; provides HttpClient with mock interceptor and TanStack Query client (devtools, 24h gcTime).
Unit-testing example: mock interceptor
examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts
New mock interceptor simulating /api/tasks GET/POST with sessionStorage persistence, delayed responses, call-number behavior, and an error path.
Unit-testing example: service + spec
examples/angular/unit-testing/src/app/services/tasks.service.ts, .../services/tasks.service.spec.ts
New TasksService (allTasks query, addTask mutation with optimistic updates/rollback and invalidate on settle) and comprehensive Jest tests exercising optimistic updates, success and failure flows.
Unit-testing example: component
examples/angular/unit-testing/src/app/components/unit-testing.component.ts
Demo component exposing injectQuery/injectMutation usage, listing tasks, adding tasks and UI indicators used by tests.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Component as UnitTestingComponent
  participant Query as allTasks (Query)
  participant Mutation as addTask (Mutation)
  participant QC as QueryClient
  participant HTTP as HttpClient
  participant INT as mockInterceptor

  Component->>Query: injectQuery(() => allTasks)
  Query->>QC: fetch ['tasks']
  QC->>HTTP: GET /api/tasks
  HTTP->>INT: intercept GET
  INT-->>HTTP: delayed 200 tasks[]
  HTTP-->>QC: tasks[]
  QC-->>Query: data updated
  Query-->>Component: status success + data

  Component->>Mutation: mutate({task})
  Mutation->>QC: onMutate -> cancel + optimistic update
  Mutation->>HTTP: POST /api/tasks
  HTTP->>INT: intercept POST
  INT-->>HTTP: delayed 201 success
  HTTP-->>Mutation: success
  Mutation->>QC: onSettled -> invalidate ['tasks']
  QC->>HTTP: GET /api/tasks (refetch)
  HTTP->>INT: intercept GET
  INT-->>HTTP: delayed 200 tasks[..., "CallNumber N"]
  HTTP-->>QC: tasks...
  QC-->>Component: cache refreshed
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested labels

package: angular-query-experimental

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat(docs): add the angular testing documentation" is concise, follows conventional commit style, and accurately summarizes the primary purpose of the changeset (adding Angular testing docs and related example material), so it is directly related and clear to reviewers.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Poem

I twitch my whiskers, tests in sight,
Mocked responses hum through the night.
I optimistically hop and then—rollback—
Jest cheers, green ticks on my stack.
A carrot for CI, a hop, a clap! 🥕🐇

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

nx-cloud bot commented Sep 11, 2025

View your CI Pipeline Execution ↗ for commit 4d17061

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ✅ Succeeded 3m 49s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1m 24s View ↗

☁️ Nx Cloud last updated this comment at 2025-09-11 23:25:27 UTC

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Nitpick comments (31)
examples/angular/unit-testing/README.md (1)

5-7: Unify test commands across package managers.

List test scripts for Yarn/PNPM/Bun like you did for install/start.

-- `npm run test` to run the tests
+- `npm test` or `yarn test` or `pnpm test` or `bun test` to run the tests
examples/angular/unit-testing/.devcontainer/devcontainer.json (1)

1-4: Bootstrap deps automatically in the devcontainer.

Add a postCreateCommand so contributors can "Open in Container" and be ready to run.

 {
   "name": "Node.js",
-  "image": "mcr.microsoft.com/devcontainers/javascript-node:22"
+  "image": "mcr.microsoft.com/devcontainers/javascript-node:22",
+  "postCreateCommand": "corepack enable && pnpm i"
 }
examples/angular/optimistic-updates/src/app/interceptor/mock-api.interceptor.ts (1)

9-9: Emit HttpErrorResponse for realistic HttpClient error handling.

Tests and app code often branch on HttpErrorResponse fields (status, url). Throwing a plain Error drops that context.

-  return throwError(() => new Error('error')).pipe(delay(1000));
+  return throwError(
+    () =>
+      new HttpErrorResponse({
+        status: 500,
+        statusText: 'Internal Server Error',
+        url: req.url,
+        error: { status: 'error' },
+      }),
+  ).pipe(delay(1000))

Add the missing import (outside the changed lines):

import { HttpResponse, HttpErrorResponse } from '@angular/common/http'

Optional: extend the header comment to note the '/api/tasks-wrong-url' error path and 1s delay.

Also applies to: 49-49

examples/angular/optimistic-updates/src/app/services/tasks.service.ts (1)

50-51: Tighten types for variables/context and align docs with onSettled invalidation.

  • Types: avoid any; reflect actual variables and context.
  • Docs: the block comment above still says “On success, invalidates…”, but invalidation now happens in onSettled.
-      onSuccess: () => {},
-      onMutate: async ({ task } : {task: string}) => {
+      onSuccess: () => {},
+      onMutate: async ({ task }: { task: string; failMutation?: boolean }) => {
@@
-      onError: (_err: any, _variables: any, context: any) => {
+      onError: (_err: unknown, _variables: { task: string; failMutation?: boolean }, context?: string[]) => {
@@
-      onSettled: () => {
-        return this.#queryClient.invalidateQueries({ queryKey: ['tasks'] })
-      },
+      onSettled: () =>
+        this.#queryClient.invalidateQueries({ queryKey: ['tasks'] }),

Nit: consider readonly on #queryClient/#http for consistency with other examples.

Also applies to: 71-71, 78-79

examples/angular/unit-testing/tsconfig.json (1)

17-23: Consider Bundler-friendly module resolution (and verbatim imports).

Angular CLI v19+ commonly works well with "bundler" resolution and "verbatimModuleSyntax" for ESM correctness. Optional, but helps avoid subtle interop surprises in tests and builds.

Apply:

-    "moduleResolution": "node",
+    "moduleResolution": "bundler",
+    "verbatimModuleSyntax": true,
examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts (2)

29-43: Minor formatting/readability around GET branch.

The closing brace after respondWith sits on the same line; reflow for clarity.

-        if (callNumber === 1) {
-        return respondWith(
+        if (callNumber === 1) {
+          return respondWith(
           200,
           JSON.parse(
             sessionStorage.getItem('unit-testing-tasks') || '[]',
-          ),
-        ) } else {
+          ),
+        )
+        } else {
           return respondWith(
             200,
             JSON.parse(
               sessionStorage.getItem('unit-testing-tasks') || '[]',
             ).concat([`CallNumber ${callNumber}`]),
           )
         }

24-26: Optional: make latency configurable for faster tests.

Allow overriding the 1000ms delay via an env flag (e.g., window.TEST_DELAY ?? 0) to speed up unit tests without changing behavior in docs/demo.

examples/angular/unit-testing/package.json (1)

23-36: Remove unused ts-node devDependency

Search found ts-node only in examples/angular/unit-testing/package.json (devDependencies) and pnpm-lock.yaml; no imports, scripts, or configs reference it — remove ts-node unless it's required by local dev tooling or CI.
Location: examples/angular/unit-testing/package.json (devDependencies).

examples/angular/unit-testing/src/app/app.component.ts (1)

8-9: Prefer explicit closing tags in Angular templates.

Self-closing component tags can be parsed, but explicit open/close improves readability and avoids tooling quirks.

-  template: `<unit-testing />`,
+  template: `<unit-testing></unit-testing>`,
examples/angular/unit-testing/src/app/app.config.ts (1)

18-26: Optional: test-friendly query defaults.

For unit-tests, disabling retries speeds failures and reduces flakiness. Consider overriding in tests (preferred) or here if this config is test-only.

Example override in tests:

provideTanStackQuery(new QueryClient({ defaultOptions: { queries: { retry: false }}}))
docs/framework/angular/guides/testing.md (7)

6-6: Clarify guidance and tighten grammar around Signals testing.

Reword for accuracy and style, and mention the official interop utility.

-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.
+Because there’s no built-in way to await a Signal reaching a specific value, use polling with Testing Library’s waitFor. Alternatively, convert Signals to Observables via toObservable (from @angular/core/rxjs-interop) and use RxJS operators to filter values.

14-14: Grammar and specificity: reference the correct package for toObservable.

-Otherwise we recommend to use the toObservable feature from Angular.
+Otherwise, we recommend using toObservable from @angular/core/rxjs-interop.

18-18: Grammar: pluralize “function”.

-Because the recommendation is to use services that provide the Query options through function this is what we are going to do.
+Because the recommendation is to use services that provide Query options through functions, this is what we’re going to do.

41-44: Docstring mismatch: the code returns a Promise, not an Observable.

- * Returns an observable containing an array of task strings.
+ * Returns a Promise that resolves to an array of task strings (TanStack Query awaits this).

49-51: Prefer firstValueFrom for single-emission HttpClient calls.

HttpClient emits once; firstValueFrom communicates intent better than lastValueFrom.

-        return lastValueFrom(this.#http.get<Array<string>>('/api/tasks'));
+        return firstValueFrom(this.#http.get<Array<string>>('/api/tasks'))

and add:

-import { lastValueFrom } from 'rxjs'
+import { firstValueFrom } from 'rxjs'

107-162: Interceptor delay is high for unit tests; consider a shorter default and a param.

A 1000 ms delay can make tests slow/flaky. Consider a smaller default (e.g., 20–50 ms) and/or a configurable delay.

-  const respondWith = (status: number, body: any) =>
-    of(new HttpResponse({ status, body })).pipe(delay(1000))
+  const delayMs = Number(sessionStorage.getItem('unit-testing-delay-ms') ?? 30)
+  const respondWith = (status: number, body: any) =>
+    of(new HttpResponse({ status, body })).pipe(delay(delayMs))

170-171: Fix typos and tighten language.

-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 live server, mock requests. There are multiple ways to do this; we recommend using an HttpInterceptor. See the Angular docs for details: https://angular.dev/guide/http/interceptors.
+You can see the interceptor setup in the “Unit testing / Jest” example.
examples/angular/unit-testing/angular.json (1)

81-82: Default build to development for faster local iteration (examples).

For an example app, consider defaulting builds to development to improve DX (faster builds, source maps by default).

-          "defaultConfiguration": "production"
+          "defaultConfiguration": "development"
examples/angular/unit-testing/src/app/components/unit-testing.component.ts (1)

15-18: Tighten wording in the intro sentence.

-      This example is the same as the optimistic-updates one but where we show how to test your service.
+      This mirrors the optimistic-updates example and demonstrates how to test the service.
examples/angular/unit-testing/src/app/services/tasks.service.ts (6)

18-21: Docstring mismatch: Promise vs Observable.

Align the comment with the code (Promise via last/firstValueFrom).

-   * Returns an observable containing an array of task strings.
+   * Returns a Promise that resolves to an array of task strings.

24-27: Prefer firstValueFrom for single-emission HttpClient.

-import { lastValueFrom } from 'rxjs'
+import { firstValueFrom } from 'rxjs'
...
-        return lastValueFrom(this.#http.get<Array<string>>('/api/tasks'));
+        return firstValueFrom(this.#http.get<Array<string>>('/api/tasks'))

31-33: Grammar nit: “refetches the”.

-   * On success, invalidates and refetch the "tasks" query cache to update the task list.
+   * On success, invalidates and refetches the "tasks" query cache to update the task list.

51-55: Comment nit: pluralize “refetches”.

-        // Cancel any outgoing refetch
+        // Cancel any outgoing refetches

57-69: Rename previousTodos to previousTasks and handle empty cache for optimistic update.

Improves clarity and enables optimistic UI even when the cache is empty.

-        const previousTodos = this.#queryClient.getQueryData<Array<string>>([
+        const previousTasks = this.#queryClient.getQueryData<Array<string>>([
           'tasks',
         ])
 
         // Optimistically update to the new value
-        if (previousTodos) {
+        if (previousTasks) {
           this.#queryClient.setQueryData<Array<string>>(
             ['tasks'],
-            [...previousTodos, task],
+            [...previousTasks, task],
           )
+        } else {
+          this.#queryClient.setQueryData<Array<string>>(['tasks'], [task])
         }
 
-        return previousTodos
+        return previousTasks

71-76: Type the error handler context for readability.

Optional, but makes rollbacks clearer.

-      onError: (_err, _variables, context) => {
+      onError: (_err: unknown, _variables: { task: string; failMutation: boolean }, context?: Array<string>) => {
         if (context) {
           // Rollback the optimistic update
           this.#queryClient.setQueryData<Array<string>>(['tasks'], context)
         }
       },
examples/angular/unit-testing/src/app/services/tasks.service.spec.ts (5)

73-74: Remove brittle pre-optimistic assertion

Immediately asserting [] right after mutateAsync races the optimistic update and can be flaky.

Apply:

-      expect(allTasks.data()).toEqual([]);
-

85-85: Optional: relax dependency on exact call number

If future tests hit the GET earlier, this strict CallNumber 2 match will break. Either keep strictness with proper resets, or relax to a pattern match.

Option A (keep strict; rely on the resets above): no change.

Option B (relax):

-      await waitFor(() => expect(allTasks.data()).toEqual([task, 'CallNumber 2']), {timeout: 10000});
+      await waitFor(
+        () => expect(allTasks.data()).toEqual([task, expect.stringMatching(/^CallNumber \d+$/)]),
+        { timeout: 10000 },
+      );

10-10: Nit: align suite name with service name

Use “TasksService” for consistency.

Apply:

-describe('Test suite: TaskService', () => {
+describe('Test suite: TasksService', () => {

45-46: Optional cleanup: clear QueryClient after each test

Prevents cache bleed and open timers when specs grow.

Apply:

+
+    afterEach(() => {
+      if (queryClient) {
+        queryClient.clear();
+      }
+    });

8-15: Type the mutation handle instead of any

Better DX and compile-time safety for onSuccess/onError payloads and variables.

Apply:

-import type { CreateQueryResult} from "@tanstack/angular-query-experimental";
+import type { CreateQueryResult, CreateMutationResult } from "@tanstack/angular-query-experimental";
@@
-    let addTask: any;
+    let addTask: CreateMutationResult<
+      { status: 'success'; task: string },
+      Error,
+      { task: string; failMutation: boolean },
+      Array<string> | undefined
+    >;
examples/angular/unit-testing/.eslintrc.cjs (1)

3-6: Empty ESLint config in examples/angular/unit-testing — mark intentional or add a minimal baseline

Verified: examples/angular/unit-testing/.eslintrc.cjs is an empty object (no root:true) and examples/angular/unit-testing/package.json does not list eslint or the suggested plugins as devDependencies.

Option A — minimal, zero new deps:

-/** @type {import('eslint').Linter.Config} */
-const config = {}
-
-module.exports = config
+/** @type {import('eslint').Linter.Config} */
+const config = {
+  root: true,
+  extends: ['eslint:recommended'],
+  env: { browser: true, node: true, es2022: true },
+  ignorePatterns: ['dist/', 'coverage/'],
+}
+
+module.exports = config

Option B — Angular/TS/Jest (requires deps: @typescript-eslint/eslint-plugin, @typescript-eslint/parser, @angular-eslint/eslint-plugin, @angular-eslint/template-parser, eslint-plugin-jest, eslint-plugin-testing-library):

-/** @type {import('eslint').Linter.Config} */
-const config = {}
-
-module.exports = config
+/** @type {import('eslint').Linter.Config} */
+const config = {
+  root: true,
+  env: { browser: true, node: true, es2022: true, jest: true },
+  extends: [
+    'eslint:recommended',
+    'plugin:@typescript-eslint/recommended',
+    'plugin:@angular-eslint/recommended',
+    'plugin:jest/recommended',
+    'plugin:testing-library/angular',
+  ],
+  parserOptions: { ecmaVersion: 'latest', sourceType: 'module' },
+  overrides: [
+    { files: ['**/*.ts'], parser: '@typescript-eslint/parser' },
+    {
+      files: ['**/*.html'],
+      parser: '@angular-eslint/template-parser',
+      extends: ['plugin:@angular-eslint/template/recommended'],
+    },
+    { files: ['**/*.spec.ts'], env: { jest: true } },
+  ],
+  ignorePatterns: ['dist/', 'coverage/'],
+}
+
+module.exports = config

I can open a follow-up PR to apply Option B and add the required devDependencies.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f0e716 and 6bba9b3.

⛔ Files ignored due to path filters (1)
  • examples/angular/unit-testing/src/favicon.ico is excluded by !**/*.ico
📒 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 (8)
examples/angular/unit-testing/src/app/app.component.ts (1)
examples/angular/unit-testing/src/app/components/unit-testing.component.ts (1)
  • Component (10-67)
examples/angular/unit-testing/jest.config.ts (1)
examples/angular/unit-testing/.eslintrc.cjs (1)
  • config (4-4)
examples/angular/unit-testing/src/app/app.config.ts (1)
examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts (1)
  • mockInterceptor (20-64)
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/main.ts (1)
examples/angular/unit-testing/src/app/app.config.ts (1)
  • appConfig (14-28)
examples/angular/unit-testing/src/app/components/unit-testing.component.ts (2)
examples/angular/unit-testing/src/app/app.component.ts (1)
  • Component (4-11)
examples/angular/optimistic-updates/src/app/components/optimistic-updates.component.ts (1)
  • Component (10-72)
examples/angular/unit-testing/src/app/services/tasks.service.spec.ts (2)
examples/angular/unit-testing/src/app/services/tasks.service.ts (1)
  • addTask (34-82)
examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts (1)
  • mockInterceptor (20-64)
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 (12)
examples/angular/unit-testing/src/index.html (1)

1-13: LGTM.

Standard Angular index; nothing to change.

examples/angular/auto-refetching/src/app/services/tasks.service.ts (1)

41-41: LGTM: returning the invalidateQueries Promise is fine.

This preserves behavior and enables consumers to await invalidation if needed.

Also applies to: 55-55

examples/angular/unit-testing/jest.config.ts (1)

1-5: Ensure Angular Jest preset and jsdom testEnvironment are configured

examples/angular/unit-testing/jest.config.ts currently only sets roots and an empty setupFilesAfterEnv — add testEnvironment: 'jsdom', preset: 'jest-preset-angular', and setupFilesAfterEnv: ['/setup-jest.ts'] (or configure these in package.json).

ripgrep under examples/angular/unit-testing returned no matches for jest-preset-angular|ts-jest|babel-jest|setup-jest — manual verification required.

docs/config.json (1)

710-713: Validate new routes exist to avoid broken nav links — verified.

docs/framework/angular/guides/testing.md and examples/angular/unit-testing/ exist; nav entry is safe.

examples/angular/unit-testing/package.json (1)

12-21: Compatibility verified — confirm repository TypeScript version

Angular 19.2.x supports TypeScript >=5.5 and <5.9 (so TS 5.8.x is valid). @testing-library/[email protected] is compatible with Angular 19.
Check examples/angular/unit-testing/package.json (and the root package.json) devDependency "typescript" and ensure it's >=5.5 <5.9; if so, no action needed.

examples/angular/unit-testing/src/main.ts (1)

1-5: LGTM.

Clean bootstrap with appConfig and proper error logging.

examples/angular/unit-testing/tsconfig.spec.json (1)

1-8: LGTM.

Spec config correctly extends base, emits to dedicated outDir, and loads Jest types.

examples/angular/unit-testing/tsconfig.app.json (1)

1-9: LGTM.

App config excludes Jest types and scopes compilation to main.ts as intended.

docs/framework/angular/guides/testing.md (1)

10-12: Keep ng add @testing-library/angular — it is supported.
Angular Testing Library provides an ng add schematic; installing as a devDependency via npm/pnpm is also a valid alternative.

Likely an incorrect or invalid review comment.

examples/angular/unit-testing/angular.json (1)

95-102: Ensure builder dependency is documented/installed.

The Jest builder requires @angular-builders/jest in devDependencies. Confirm the example’s package.json includes it (with matching major to Angular), or add a note to the docs.

If missing, add:

 "devDependencies": {
+  "@angular-builders/jest": "^18.0.0"
 }
examples/angular/unit-testing/src/app/services/tasks.service.spec.ts (1)

24-30: Good: retries disabled in tests

Disabling query retries eliminates timing flakiness and speeds up failures. LGTM.

examples/angular/unit-testing/.eslintrc.cjs (1)

1-1: LGTM: keep // @ts-check.

Good editor DX; no issues.

Comment on lines 90 to 104
it('should get all the Tasks', () => {
let allTasks: any;
runInInjectionContext(injector, () => {
allTasks = injectQuery(() => service.allTasks());
});
expect(allTasks.status()).toEqual('pending');
expect(allTasks.isFetching()).toEqual(true);
expect(allTasks.data()).toEqual(undefined);
// 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.
// To have a more complete example have a look at "unit testing / jest"
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Test must be async to use await; fix minor typos.

The spec uses await but the test callback isn’t async. Also fix “inteceptor” → “interceptor”.

-    it('should get all the Tasks', () => {
+    it('should get all the tasks', async () => {
       let allTasks: any;
       runInInjectionContext(injector, () => {
         allTasks = injectQuery(() => service.allTasks());
       });
       expect(allTasks.status()).toEqual('pending');
       expect(allTasks.isFetching()).toEqual(true);
       expect(allTasks.data()).toEqual(undefined);
       // 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.
+      expect(allTasks.data()).toEqual([]); // The interceptor returns [] on the first query request.
       // To have a more complete example have a look at "unit testing / jest"
     });
📝 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.

Suggested change
it('should get all the Tasks', () => {
let allTasks: any;
runInInjectionContext(injector, () => {
allTasks = injectQuery(() => service.allTasks());
});
expect(allTasks.status()).toEqual('pending');
expect(allTasks.isFetching()).toEqual(true);
expect(allTasks.data()).toEqual(undefined);
// 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.
// To have a more complete example have a look at "unit testing / jest"
});
});
it('should get all the tasks', async () => {
let allTasks: any;
runInInjectionContext(injector, () => {
allTasks = injectQuery(() => service.allTasks());
});
expect(allTasks.status()).toEqual('pending');
expect(allTasks.isFetching()).toEqual(true);
expect(allTasks.data()).toEqual(undefined);
// 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([]); // The interceptor returns [] on the first query request.
// To have a more complete example have a look at "unit testing / jest"
});
});
🤖 Prompt for AI Agents
In docs/framework/angular/guides/testing.md around lines 90 to 104, the spec
uses await but the test callback is not declared async and there's a typo
"inteceptor"; fix by changing the test definition to an async function (e.g.,
it('should get all the Tasks', async () => { ... })) so await works, and correct
the typo "inteceptor" to "interceptor" in the comment; ensure any use of await
is inside the newly async callback.

Comment on lines 1 to 5
const config: import('jest').Config = {
roots: ['<rootDir>/src/'],
setupFilesAfterEnv: []
};
export default config; No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix ESLint violation: prefer named type import over import() annotation.

Resolves @typescript-eslint/consistent-type-imports.

-const config: import('jest').Config = {
-    roots: ['<rootDir>/src/'],
-    setupFilesAfterEnv: []
-  };
-  export default config;
+import type { Config } from 'jest'
+
+const config: Config = {
+  roots: ['<rootDir>/src/'],
+  setupFilesAfterEnv: [],
+}
+export default config
📝 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.

Suggested change
const config: import('jest').Config = {
roots: ['<rootDir>/src/'],
setupFilesAfterEnv: []
};
export default config;
import type { Config } from 'jest'
const config: Config = {
roots: ['<rootDir>/src/'],
setupFilesAfterEnv: [],
}
export default config
🧰 Tools
🪛 ESLint

[error] 1-1: import() type annotations are forbidden.

(@typescript-eslint/consistent-type-imports)

🤖 Prompt for AI Agents
In examples/angular/unit-testing/jest.config.ts around lines 1 to 5, the file
uses the import() type annotation which triggers the
@typescript-eslint/consistent-type-imports rule; change it to a named type
import: add "import type { Config } from 'jest';" at the top and replace "const
config: import('jest').Config" with "const config: Config" so the type is
imported as a named type import and ESLint no longer flags the file.

Comment on lines +31 to +36
"@types/jest": "^29.5.12",
"@types/node": "^12.11.1",
"jest": "^29.7.0",
"ts-node": "~10.8.1",
"typescript": "5.8.2"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Update @types/node to a modern major.

The repo targets Angular 19 and Jest; Node v12 typings are very outdated and can cause type conflicts. Align with your workspace Node LTS (commonly 20 or 22).

-    "@types/node": "^12.11.1",
+    "@types/node": "^20",
📝 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.

Suggested change
"@types/jest": "^29.5.12",
"@types/node": "^12.11.1",
"jest": "^29.7.0",
"ts-node": "~10.8.1",
"typescript": "5.8.2"
}
"@types/jest": "^29.5.12",
"@types/node": "^20",
"jest": "^29.7.0",
"ts-node": "~10.8.1",
"typescript": "5.8.2"
}
🤖 Prompt for AI Agents
In examples/angular/unit-testing/package.json around lines 31 to 36, the
dependency "@types/node": "^12.11.1" is outdated and can conflict with Angular
19 and recent Jest; update the @types/node entry to match your workspace Node
LTS major (for example ^20.0.0 or ^22.0.0), then run your package manager
install and re-run the test/typecheck to confirm no remaining type conflicts; if
conflicts appear, adjust tsconfig's lib/skipLibCheck or update other devDeps to
compatible versions.

Comment on lines 10 to 48
@Component({
changeDetection: ChangeDetectionStrategy.OnPush,
selector: 'unit-testing',
imports: [FormsModule, DatePipe],
template: `
<p>
This example is the same as the optimistic-updates one but where we show how to test your service.
</p>

<hr />
@if (tasks.isLoading()) {
<p>Loading...</p>
}

<div class="container">
<label>
<input type="checkbox" [(ngModel)]="failMutation" />
Fail Mutation
</label>

<div class="input-container">
<input type="text" [(ngModel)]="newItem" placeholder="Enter text" />
<button (click)="addItem()">Create</button>
<ul>
@for (task of tasks.data(); track task) {
<li>{{ task }}</li>
}
</ul>

<div>
Updated At: {{ tasks.dataUpdatedAt() | date: 'MMMM d, h:mm:ss a ' }}
</div>
</div>
@if (!tasks.isLoading() && tasks.isFetching()) {
<p>Fetching in background</p>
}
</div>
`,
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing standalone: true on a component that is imported as standalone.

AppComponent imports UnitTestingComponent directly, so this component must be standalone.

 @Component({
   changeDetection: ChangeDetectionStrategy.OnPush,
   selector: 'unit-testing',
+  standalone: true,
   imports: [FormsModule, DatePipe],
📝 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.

Suggested change
@Component({
changeDetection: ChangeDetectionStrategy.OnPush,
selector: 'unit-testing',
imports: [FormsModule, DatePipe],
template: `
<p>
This example is the same as the optimistic-updates one but where we show how to test your service.
</p>
<hr />
@if (tasks.isLoading()) {
<p>Loading...</p>
}
<div class="container">
<label>
<input type="checkbox" [(ngModel)]="failMutation" />
Fail Mutation
</label>
<div class="input-container">
<input type="text" [(ngModel)]="newItem" placeholder="Enter text" />
<button (click)="addItem()">Create</button>
<ul>
@for (task of tasks.data(); track task) {
<li>{{ task }}</li>
}
</ul>
<div>
Updated At: {{ tasks.dataUpdatedAt() | date: 'MMMM d, h:mm:ss a ' }}
</div>
</div>
@if (!tasks.isLoading() && tasks.isFetching()) {
<p>Fetching in background</p>
}
</div>
`,
})
@Component({
changeDetection: ChangeDetectionStrategy.OnPush,
selector: 'unit-testing',
standalone: true,
imports: [FormsModule, DatePipe],
template: `
<p>
This example is the same as the optimistic-updates one but where we show how to test your service.
</p>
<hr />
@if (tasks.isLoading()) {
<p>Loading...</p>
}
<div class="container">
<label>
<input type="checkbox" [(ngModel)]="failMutation" />
Fail Mutation
</label>
<div class="input-container">
<input type="text" [(ngModel)]="newItem" placeholder="Enter text" />
<button (click)="addItem()">Create</button>
<ul>
@for (task of tasks.data(); track task) {
<li>{{ task }}</li>
}
</ul>
<div>
Updated At: {{ tasks.dataUpdatedAt() | date: 'MMMM d, h:mm:ss a ' }}
</div>
</div>
@if (!tasks.isLoading() && tasks.isFetching()) {
<p>Fetching in background</p>
}
</div>
`,
})
🤖 Prompt for AI Agents
examples/angular/unit-testing/src/app/components/unit-testing.component.ts lines
10-48: The component is used as a standalone import but the @Component decorator
is missing standalone: true; add standalone: true to the decorator metadata so
the component can be imported directly without being declared in an NgModule,
and ensure any imports (FormsModule, DatePipe) remain listed in the imports
array.

Comment on lines 44 to 57
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,
})
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix const declaration inside switch-case (noSwitchDeclarations).

Wrap the POST case body in a block to satisfy lint/correctness and avoid scope bleed across cases.

-      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,
+        })
+      }
📝 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.

Suggested change
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 POST case declares const variables directly inside
the switch which can leak scope; wrap the entire POST case body in a block (add
{ ... } after case 'POST': and before the return) so the const declarations are
scoped to that case and do not bleed into other cases.

Comment on lines 33 to 35
service = TestBed.inject(TasksService);
injector = TestBed.inject(Injector);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Ensure test isolation by clearing mock storage between tests

The interceptor uses sessionStorage to store tasks. Clear it in beforeEach to keep the initial dataset deterministic.

Apply:

       service = TestBed.inject(TasksService);
       injector = TestBed.inject(Injector);
 
+      // Ensure interceptor-backed storage starts clean
+      sessionStorage.removeItem('unit-testing-tasks');
📝 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.

Suggested change
service = TestBed.inject(TasksService);
injector = TestBed.inject(Injector);
service = TestBed.inject(TasksService);
injector = TestBed.inject(Injector);
// Ensure interceptor-backed storage starts clean
sessionStorage.removeItem('unit-testing-tasks');
🤖 Prompt for AI Agents
In examples/angular/unit-testing/src/app/services/tasks.service.spec.ts around
lines 33 to 35, tests rely on sessionStorage state set by an interceptor so you
must ensure isolation by clearing sessionStorage before each test; update the
test setup to call sessionStorage.clear() (or remove the specific key used by
the interceptor) inside the beforeEach so each test starts with a deterministic
initial dataset, and ensure any setup that repopulates storage runs after the
clear.

Comment on lines 36 to 44
runInInjectionContext(injector, () => {
allTasks = injectQuery(() => service.allTasks());
addTask = injectMutation(() => service.addTask());
queryClient = inject(QueryClient);
});
expect(allTasks.status()).toEqual('pending');
expect(allTasks.isFetching()).toEqual(true);
expect(allTasks.data()).toEqual(undefined);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix: avoid unintended GET in beforeEach that breaks the next test’s initial-state assumptions

Injecting the query/mutation here triggers a GET via the mock interceptor and increments its internal counter. The subsequent test then starts at callNumber=2 and allTasks.data() is not [] anymore, causing deterministic failures. Move this injection into the specific test that needs it.

Apply:

-      runInInjectionContext(injector, () => {
-        allTasks = injectQuery(() => service.allTasks());
-        addTask = injectMutation(() => service.addTask());
-        queryClient = inject(QueryClient);
-      });
-      expect(allTasks.status()).toEqual('pending');
-      expect(allTasks.isFetching()).toEqual(true);
-      expect(allTasks.data()).toEqual(undefined);
📝 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.

Suggested change
runInInjectionContext(injector, () => {
allTasks = injectQuery(() => service.allTasks());
addTask = injectMutation(() => service.addTask());
queryClient = inject(QueryClient);
});
expect(allTasks.status()).toEqual('pending');
expect(allTasks.isFetching()).toEqual(true);
expect(allTasks.data()).toEqual(undefined);
});
});
🤖 Prompt for AI Agents
In examples/angular/unit-testing/src/app/services/tasks.service.spec.ts around
lines 36 to 44, the test setup is injecting the query/mutation inside a shared
beforeEach which causes an unintended GET via the mock interceptor and
increments its call counter, breaking the next test's expected initial state;
move the runInInjectionContext(...) block (the
injectQuery/injectMutation/inject(QueryClient) calls) out of the shared
beforeEach and into only the individual test(s) that require those injections so
the mock interceptor call count and initial allTasks.data() remain deterministic
for other tests.

Comment on lines 51 to 56
it('should manage all tasks, add task, remove task', async () => {
// 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([]);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Inject query/mutation inside the test that needs them (and assert initial state there)

This keeps the “service creation” test from mutating interceptor state and makes the initial-state assertions local and reliable.

Apply:

-    it('should manage all tasks, add task, remove task', async () => {
-      // We await the first result from the query
+    it('should manage all tasks and add task (optimistic update + rollback)', async () => {
+      runInInjectionContext(injector, () => {
+        allTasks = injectQuery(() => service.allTasks());
+        addTask = injectMutation(() => service.addTask());
+        queryClient = inject(QueryClient);
+      });
+      // Initial state
+      expect(allTasks.status()).toEqual('pending');
+      expect(allTasks.isFetching()).toEqual(true);
+      expect(allTasks.data()).toEqual(undefined);
+      // 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([]);
📝 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.

Suggested change
it('should manage all tasks, add task, remove task', async () => {
// 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([]);
it('should manage all tasks and add task (optimistic update + rollback)', async () => {
runInInjectionContext(injector, () => {
allTasks = injectQuery(() => service.allTasks());
addTask = injectMutation(() => service.addTask());
queryClient = inject(QueryClient);
});
// Initial state
expect(allTasks.status()).toEqual('pending');
expect(allTasks.isFetching()).toEqual(true);
expect(allTasks.data()).toEqual(undefined);
// 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([]);

Copy link

pkg-pr-new bot commented Sep 11, 2025

More templates

@tanstack/angular-query-devtools-experimental

npm i https://pkg.pr.new/@tanstack/angular-query-devtools-experimental@9023

@tanstack/angular-query-experimental

npm i https://pkg.pr.new/@tanstack/angular-query-experimental@9023

@tanstack/eslint-plugin-query

npm i https://pkg.pr.new/@tanstack/eslint-plugin-query@9023

@tanstack/query-async-storage-persister

npm i https://pkg.pr.new/@tanstack/query-async-storage-persister@9023

@tanstack/query-broadcast-client-experimental

npm i https://pkg.pr.new/@tanstack/query-broadcast-client-experimental@9023

@tanstack/query-core

npm i https://pkg.pr.new/@tanstack/query-core@9023

@tanstack/query-devtools

npm i https://pkg.pr.new/@tanstack/query-devtools@9023

@tanstack/query-persist-client-core

npm i https://pkg.pr.new/@tanstack/query-persist-client-core@9023

@tanstack/query-sync-storage-persister

npm i https://pkg.pr.new/@tanstack/query-sync-storage-persister@9023

@tanstack/react-query

npm i https://pkg.pr.new/@tanstack/react-query@9023

@tanstack/react-query-devtools

npm i https://pkg.pr.new/@tanstack/react-query-devtools@9023

@tanstack/react-query-next-experimental

npm i https://pkg.pr.new/@tanstack/react-query-next-experimental@9023

@tanstack/react-query-persist-client

npm i https://pkg.pr.new/@tanstack/react-query-persist-client@9023

@tanstack/solid-query

npm i https://pkg.pr.new/@tanstack/solid-query@9023

@tanstack/solid-query-devtools

npm i https://pkg.pr.new/@tanstack/solid-query-devtools@9023

@tanstack/solid-query-persist-client

npm i https://pkg.pr.new/@tanstack/solid-query-persist-client@9023

@tanstack/svelte-query

npm i https://pkg.pr.new/@tanstack/svelte-query@9023

@tanstack/svelte-query-devtools

npm i https://pkg.pr.new/@tanstack/svelte-query-devtools@9023

@tanstack/svelte-query-persist-client

npm i https://pkg.pr.new/@tanstack/svelte-query-persist-client@9023

@tanstack/vue-query

npm i https://pkg.pr.new/@tanstack/vue-query@9023

@tanstack/vue-query-devtools

npm i https://pkg.pr.new/@tanstack/vue-query-devtools@9023

commit: 4d17061

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (2)
examples/angular/unit-testing/jest.config.ts (1)

1-5: Fix ESLint violation: use a named type import instead of import() annotation.

Aligns with @typescript-eslint/consistent-type-imports and unblocks lint. Same as the prior bot comment.

+import type { Config } from 'jest'
-const config: import('jest').Config = {
+const config: Config = {
   roots: ['<rootDir>/src/'],
   setupFilesAfterEnv: [],
 }
 export default config
examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts (1)

43-52: Fix const declaration inside switch-case (noSwitchDeclarations).

Wrap the POST case body in a block to satisfy Biome and prevent scope bleed across cases.

-      case 'POST':
+      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,
         })
+      }
🧹 Nitpick comments (6)
examples/angular/unit-testing/jest.config.ts (1)

1-5: Optional: prefer “satisfies” for better inference.

Keeps literal types while enforcing the Config shape (TS ≥4.9).

-import type { Config } from 'jest'
-const config: Config = {
+import type { Config } from 'jest'
+const config = {
   roots: ['<rootDir>/src/'],
   setupFilesAfterEnv: [],
-}
+} satisfies Config
 export default config
examples/angular/unit-testing/src/app/interceptor/mock-api.interceptor.ts (5)

27-53: Add a default branch to forward unsupported methods.

Makes intent explicit and avoids accidental fall-through if new methods are added later.

       switch (req.method) {
         case 'GET':
           ...
-        } 
-      case 'POST': {
+        }
+        case 'POST': {
           ...
         }
+        default:
+          return next(req)
       }

18-19: Persist call counter in sessionStorage to avoid cross-test leakage; scope GET case.

Module-level callNumber can leak across tests/processes. Persist it so sessionStorage.clear() in tests fully resets state. Also wrap GET body in a block when introducing a const.

- let callNumber = 0
+ const CALL_KEY = 'unit-testing-callNumber'
-      case 'GET':
-        call_number++
+      case 'GET': {
+        const callNumber =
+          Number(sessionStorage.getItem(CALL_KEY) ?? '0') + 1
+        sessionStorage.setItem(CALL_KEY, String(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}`]),
           )
         }
+      }

Also applies to: 28-42


33-40: Extract the storage key to a constant (DRY, fewer typos).

-            JSON.parse(sessionStorage.getItem('unit-testing-tasks') || '[]'),
+            JSON.parse(sessionStorage.getItem(STORAGE_KEY) || '[]'),
-        const tasks = JSON.parse(
-          sessionStorage.getItem('unit-testing-tasks') || '[]',
-        )
+        const tasks = JSON.parse(sessionStorage.getItem(STORAGE_KEY) || '[]')
-        sessionStorage.setItem('unit-testing-tasks', JSON.stringify(tasks))
+        sessionStorage.setItem(STORAGE_KEY, JSON.stringify(tasks))

Add near the top (after line 18) to support the change:

const STORAGE_KEY = 'unit-testing-tasks'

Also applies to: 45-48


24-26: Make the artificial delay configurable via request header.

Speeds up or disables delay in tests without code changes.

-  const respondWith = (status: number, body: any) =>
-    of(new HttpResponse({ status, body })).pipe(delay(1000))
+  const hdr = req.headers.get('x-mock-delay-ms')
+  const delayMs = hdr !== null && !Number.isNaN(+hdr) ? +hdr : 1000
+  const respondWith = (status: number, body: any) =>
+    of(new HttpResponse({ status, body })).pipe(delay(delayMs))
-    return throwError(() => new Error('error')).pipe(delay(1000))
+    return throwError(() => new Error('error')).pipe(delay(delayMs))

Also applies to: 55-57


1-7: Docblock naming: align with exported symbol.

Rename “MockApiInterceptor” in the comment to “mockInterceptor” (or “Mock API interceptor”) for consistency.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6bba9b3 and 4d17061.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (9)
  • docs/framework/angular/guides/testing.md (1 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/jest.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/tsconfig.spec.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
  • examples/angular/unit-testing/src/app/components/unit-testing.component.ts
  • examples/angular/unit-testing/src/app/services/tasks.service.ts
  • examples/angular/optimistic-updates/src/app/interceptor/mock-api.interceptor.ts
  • examples/angular/unit-testing/src/app/services/tasks.service.spec.ts
  • examples/angular/optimistic-updates/src/app/services/tasks.service.ts
  • examples/angular/unit-testing/tsconfig.spec.json
  • docs/framework/angular/guides/testing.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-19T03:18:18.303Z
Learnt from: oscartbeaumont
PR: TanStack/query#9564
File: packages/solid-query-devtools/src/production.tsx:2-3
Timestamp: 2025-08-19T03:18:18.303Z
Learning: In the solid-query-devtools package, the codebase uses a pattern of type-only default imports combined with typeof for component type annotations (e.g., `import type SolidQueryDevtoolsComp from './devtools'` followed by `typeof SolidQueryDevtoolsComp`). This pattern is consistently used across index.tsx and production.tsx files, and the maintainers prefer consistency over changing this approach.

Applied to files:

  • examples/angular/unit-testing/jest.config.ts
🧬 Code graph analysis (1)
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)
🪛 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] 44-46: 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 (1)
examples/angular/unit-testing/jest.config.ts (1)

1-4: Optional: set testEnvironment to 'jsdom' for Angular tests.

jest-preset-angular only appears in pnpm-lock.yaml — no preset reference found in configs for this example. If the example doesn't inherit the preset, add testEnvironment: 'jsdom' to examples/angular/unit-testing/jest.config.ts; otherwise ignore.

Copy link

codecov bot commented Sep 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.51%. Comparing base (7f0e716) to head (4d17061).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #9023   +/-   ##
=======================================
  Coverage   45.51%   45.51%           
=======================================
  Files         209      209           
  Lines        8379     8379           
  Branches     1896     1897    +1     
=======================================
  Hits         3814     3814           
  Misses       4118     4118           
  Partials      447      447           
Components Coverage Δ
@tanstack/angular-query-devtools-experimental ∅ <ø> (∅)
@tanstack/angular-query-experimental 87.00% <ø> (ø)
@tanstack/eslint-plugin-query 83.24% <ø> (ø)
@tanstack/query-async-storage-persister 43.85% <ø> (ø)
@tanstack/query-broadcast-client-experimental 24.39% <ø> (ø)
@tanstack/query-codemods 0.00% <ø> (ø)
@tanstack/query-core 97.48% <ø> (ø)
@tanstack/query-devtools 3.48% <ø> (ø)
@tanstack/query-persist-client-core 79.60% <ø> (ø)
@tanstack/query-sync-storage-persister 84.61% <ø> (ø)
@tanstack/query-test-utils 77.77% <ø> (ø)
@tanstack/react-query 96.00% <ø> (ø)
@tanstack/react-query-devtools 10.00% <ø> (ø)
@tanstack/react-query-next-experimental ∅ <ø> (∅)
@tanstack/react-query-persist-client 100.00% <ø> (ø)
@tanstack/solid-query 78.13% <ø> (ø)
@tanstack/solid-query-devtools ∅ <ø> (∅)
@tanstack/solid-query-persist-client 100.00% <ø> (ø)
@tanstack/svelte-query 87.58% <ø> (ø)
@tanstack/svelte-query-devtools ∅ <ø> (∅)
@tanstack/svelte-query-persist-client 100.00% <ø> (ø)
@tanstack/vue-query 71.10% <ø> (ø)
@tanstack/vue-query-devtools ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants