Angular v19+ pending tasks and lazy derived signal proxy initialization #9986
Angular v19+ pending tasks and lazy derived signal proxy initialization #9986benjavicente wants to merge 28 commits intoTanStack:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 55b78eb The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
📝 WalkthroughWalkthroughUpdates the Angular experimental package to require Angular v19+, remove a pending-tasks compat shim in favor of Angular's PendingTasks, add method-exclusion support to signal proxies, refactor createBaseQuery and inject-* lifecycles, introduce injectQueries (types + runtime + docs), consolidate test utilities, and update many docs/examples and package metadata. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Component as Angular Component
participant InjectAPI as injectQuery / injectMutation / injectQueries
participant BaseQuery as createBaseQuery
participant Pending as PendingTasks
participant QueryClient as QueryClient / Observer
Note over Component,InjectAPI: component requests a reactive query/mutation/queries
Component->>InjectAPI: call injectQuery / injectMutation / injectQueries(options)
InjectAPI->>BaseQuery: createBaseQuery(options, methodsToExclude)
BaseQuery->>QueryClient: create Observer / subscribe
BaseQuery->>Pending: on pending start -> Pending.add()
Pending-->>BaseQuery: returns cleanup handle
QueryClient-->>BaseQuery: notify updates (data/error/status)
BaseQuery->>Component: expose signal-proxied result (excluded methods wrapped/untracked)
Component->>BaseQuery: invoke exposed method (e.g., refetch)
alt method excluded
Component->>QueryClient: method invoked via untracked wrapper
else method proxied as signal
Component->>BaseQuery: react to proxied signal updates
end
Note over BaseQuery,Pending: on destroy -> call cleanup to stop pending task
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
documentation
|
Changed back |
![coderabbitai[bot]](https://avatars-githubusercontent-com-gh.computerqwq.top/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
.changeset/deep-crews-open.md (1)
5-5: Consider expanding the changeset message for clarity.The message is brief. Consider adding more detail about the key changes (e.g., signal proxy improvements, PendingTasks integration, or removal of Angular <19 compatibility) to help consumers understand the scope of the update.
packages/angular-query-experimental/src/__tests__/with-devtools.test.ts (1)
138-138: LGTM! Consistent test utility usage.Replacing direct timer advancement with
flushQueryUpdates()provides better abstraction and makes the test suite more maintainable. This utility wraps the timer advancement logic in a semantic function name that clearly communicates intent.Also applies to: 170-170, 190-190, 206-206, 251-251, 291-291, 323-323, 354-354, 387-387, 417-417, 443-443, 473-473, 530-530, 551-551, 581-581
packages/angular-query-experimental/src/__tests__/inject-devtools-panel.test.ts (1)
39-41: LGTM! Cleaner test setup.Using
setupTanStackQueryTestBedconsolidates common TestBed configuration while allowing test-specific providers (ElementRef) to be added cleanly. This improves maintainability and reduces duplication across the test suite.docs/framework/angular/quick-start.md (1)
38-38: Minor inconsistency in provider ordering between examples.The NgModule example has
provideTanStackQuerybeforeprovideHttpClient, while thebootstrapApplicationexample on line 22 has the opposite order. Consider aligning the order for consistency across examples.🔎 Suggested fix
- providers: [provideTanStackQuery(new QueryClient()), provideHttpClient()], + providers: [provideHttpClient(), provideTanStackQuery(new QueryClient())],packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts (1)
540-588: Consider extracting TestBed reset pattern.Multiple tests repeat the
TestBed.resetTestingModule()+TestBed.configureTestingModule()pattern with identical providers. Consider using thesetupTanStackQueryTestBedutility fromtest-utils.tsto reduce duplication.🔎 Proposed refactor
test('should handle synchronous mutation with retry', async () => { - TestBed.resetTestingModule() - TestBed.configureTestingModule({ - providers: [ - provideZonelessChangeDetection(), - provideTanStackQuery(queryClient), - ], - }) + setupTanStackQueryTestBed(queryClient) const app = TestBed.inject(ApplicationRef)Apply similar changes to other tests that reset the TestBed.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (57)
.changeset/deep-crews-open.mddocs/framework/angular/angular-httpclient-and-other-data-fetching-clients.mddocs/framework/angular/guides/caching.mddocs/framework/angular/guides/default-query-function.mddocs/framework/angular/guides/dependent-queries.mddocs/framework/angular/guides/disabling-queries.mddocs/framework/angular/guides/does-this-replace-client-state.mddocs/framework/angular/guides/important-defaults.mddocs/framework/angular/guides/infinite-queries.mddocs/framework/angular/guides/initial-query-data.mddocs/framework/angular/guides/invalidations-from-mutations.mddocs/framework/angular/guides/mutations.mddocs/framework/angular/guides/paginated-queries.mddocs/framework/angular/guides/parallel-queries.mddocs/framework/angular/guides/placeholder-query-data.mddocs/framework/angular/guides/queries.mddocs/framework/angular/guides/query-functions.mddocs/framework/angular/guides/query-invalidation.mddocs/framework/angular/guides/query-options.mddocs/framework/angular/guides/query-retries.mddocs/framework/angular/guides/testing.mddocs/framework/angular/installation.mddocs/framework/angular/overview.mddocs/framework/angular/quick-start.mddocs/framework/angular/reference/functions/injectQuery.mddocs/framework/angular/typescript.mdexamples/angular/infinite-query-with-max-pages/src/app/components/example.component.tsexamples/angular/optimistic-updates/src/app/components/optimistic-updates.component.tspackages/angular-query-experimental/README.mdpackages/angular-query-experimental/package.jsonpackages/angular-query-experimental/src/__tests__/inject-devtools-panel.test.tspackages/angular-query-experimental/src/__tests__/inject-infinite-query.test-d.tspackages/angular-query-experimental/src/__tests__/inject-infinite-query.test.tspackages/angular-query-experimental/src/__tests__/inject-is-fetching.test.tspackages/angular-query-experimental/src/__tests__/inject-is-mutating.test.tspackages/angular-query-experimental/src/__tests__/inject-mutation-state.test.tspackages/angular-query-experimental/src/__tests__/inject-mutation.test.tspackages/angular-query-experimental/src/__tests__/inject-queries.test-d.tspackages/angular-query-experimental/src/__tests__/inject-queries.test.tspackages/angular-query-experimental/src/__tests__/inject-query.test-d.tspackages/angular-query-experimental/src/__tests__/inject-query.test.tspackages/angular-query-experimental/src/__tests__/mutation-options.test.tspackages/angular-query-experimental/src/__tests__/pending-tasks-ssr.test.tspackages/angular-query-experimental/src/__tests__/pending-tasks.test.tspackages/angular-query-experimental/src/__tests__/signal-proxy.test.tspackages/angular-query-experimental/src/__tests__/test-utils.tspackages/angular-query-experimental/src/__tests__/with-devtools.test.tspackages/angular-query-experimental/src/create-base-query.tspackages/angular-query-experimental/src/inject-infinite-query.tspackages/angular-query-experimental/src/inject-mutation.tspackages/angular-query-experimental/src/inject-queries.tspackages/angular-query-experimental/src/inject-query.tspackages/angular-query-experimental/src/pending-tasks-compat.tspackages/angular-query-experimental/src/query-options.tspackages/angular-query-experimental/src/signal-proxy.tspackages/angular-query-experimental/src/types.tspackages/angular-query-persist-client/package.json
💤 Files with no reviewable changes (4)
- docs/framework/angular/guides/important-defaults.md
- docs/framework/angular/angular-httpclient-and-other-data-fetching-clients.md
- packages/angular-query-persist-client/package.json
- packages/angular-query-experimental/src/pending-tasks-compat.ts
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: arnoud-dv
Repo: TanStack/query PR: 9669
File: docs/framework/angular/guides/testing.md:49-56
Timestamp: 2025-09-21T00:31:02.518Z
Learning: TestBed.tick() is a valid API introduced in Angular 20 for triggering effects in unit tests, similar to ApplicationRef.tick() but for testing contexts.
📚 Learning: 2025-11-22T09:06:05.219Z
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
Applied to files:
packages/angular-query-experimental/src/__tests__/pending-tasks-ssr.test.tspackages/angular-query-experimental/src/__tests__/inject-infinite-query.test-d.tspackages/angular-query-experimental/src/__tests__/inject-devtools-panel.test.tspackages/angular-query-experimental/src/__tests__/test-utils.tspackages/angular-query-experimental/src/__tests__/inject-query.test-d.tsdocs/framework/angular/reference/functions/injectQuery.mdpackages/angular-query-experimental/src/__tests__/inject-queries.test-d.ts.changeset/deep-crews-open.mddocs/framework/angular/guides/query-functions.mdpackages/angular-query-experimental/src/__tests__/with-devtools.test.tsdocs/framework/angular/typescript.mddocs/framework/angular/overview.mdpackages/angular-query-experimental/src/__tests__/inject-mutation.test.tsdocs/framework/angular/guides/query-invalidation.mddocs/framework/angular/guides/query-retries.mddocs/framework/angular/guides/testing.mdpackages/angular-query-experimental/src/__tests__/inject-is-fetching.test.tspackages/angular-query-experimental/src/__tests__/inject-is-mutating.test.tspackages/angular-query-experimental/src/inject-mutation.tspackages/angular-query-experimental/src/__tests__/mutation-options.test.tspackages/angular-query-experimental/src/create-base-query.tsdocs/framework/angular/guides/disabling-queries.mddocs/framework/angular/guides/initial-query-data.mdpackages/angular-query-experimental/src/__tests__/inject-query.test.tspackages/angular-query-experimental/src/__tests__/inject-mutation-state.test.tspackages/angular-query-experimental/src/__tests__/inject-queries.test.tspackages/angular-query-experimental/src/__tests__/inject-infinite-query.test.tspackages/angular-query-experimental/src/__tests__/pending-tasks.test.tsdocs/framework/angular/guides/dependent-queries.mdpackages/angular-query-experimental/src/query-options.tsdocs/framework/angular/guides/parallel-queries.md
📚 Learning: 2025-09-21T00:31:02.518Z
Learnt from: arnoud-dv
Repo: TanStack/query PR: 9669
File: docs/framework/angular/guides/testing.md:49-56
Timestamp: 2025-09-21T00:31:02.518Z
Learning: TestBed.tick() is a valid API introduced in Angular 20 for triggering effects in unit tests, similar to ApplicationRef.tick() but for testing contexts.
Applied to files:
packages/angular-query-experimental/src/__tests__/pending-tasks-ssr.test.tspackages/angular-query-experimental/src/__tests__/inject-devtools-panel.test.tspackages/angular-query-experimental/src/__tests__/test-utils.ts.changeset/deep-crews-open.mdpackages/angular-query-experimental/src/__tests__/with-devtools.test.tspackages/angular-query-experimental/src/__tests__/signal-proxy.test.tspackages/angular-query-experimental/src/__tests__/inject-mutation.test.tsdocs/framework/angular/guides/testing.mdpackages/angular-query-experimental/src/__tests__/inject-is-fetching.test.tspackages/angular-query-experimental/src/__tests__/inject-is-mutating.test.tspackages/angular-query-experimental/src/__tests__/mutation-options.test.tspackages/angular-query-experimental/src/__tests__/inject-query.test.tspackages/angular-query-experimental/src/__tests__/inject-mutation-state.test.tspackages/angular-query-experimental/src/__tests__/inject-queries.test.tspackages/angular-query-experimental/src/__tests__/inject-infinite-query.test.tspackages/angular-query-experimental/src/__tests__/pending-tasks.test.ts
📚 Learning: 2025-11-02T22:52:33.071Z
Learnt from: DogPawHat
Repo: TanStack/query PR: 9835
File: packages/query-core/src/__tests__/queryClient.test-d.tsx:242-256
Timestamp: 2025-11-02T22:52:33.071Z
Learning: In the TanStack Query codebase, the new `query` and `infiniteQuery` methods support the `select` option for data transformation, while the legacy `fetchQuery` and `fetchInfiniteQuery` methods do not support `select` and should reject it at the type level.
Applied to files:
packages/angular-query-experimental/src/__tests__/inject-infinite-query.test-d.tspackages/angular-query-experimental/src/__tests__/inject-query.test-d.tsdocs/framework/angular/guides/query-functions.mddocs/framework/angular/overview.mddocs/framework/angular/guides/query-retries.mdpackages/angular-query-experimental/src/inject-infinite-query.tspackages/angular-query-experimental/src/inject-queries.tsdocs/framework/angular/guides/disabling-queries.mdpackages/angular-query-experimental/src/__tests__/inject-query.test.tspackages/angular-query-experimental/src/inject-query.tspackages/angular-query-experimental/src/query-options.tspackages/angular-query-experimental/src/types.tsdocs/framework/angular/guides/query-options.md
📚 Learning: 2025-09-02T17:57:33.184Z
Learnt from: TkDodo
Repo: TanStack/query PR: 9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.184Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
Applied to files:
packages/angular-query-experimental/package.jsondocs/framework/angular/guides/query-retries.mddocs/framework/angular/guides/testing.mdpackages/angular-query-experimental/src/inject-mutation.tspackages/angular-query-experimental/src/create-base-query.ts
📚 Learning: 2025-08-25T19:16:59.365Z
Learnt from: justjake
Repo: TanStack/query PR: 9589
File: packages/query-core/src/queryCache.ts:132-146
Timestamp: 2025-08-25T19:16:59.365Z
Learning: The TypeScript pattern `t satisfies never` in a switch statement's default case is valid for exhaustiveness checking when all possible values of a union type are handled in the switch cases. For `typeof` operations, this ensures all 8 possible return values are covered: "string", "number", "bigint", "boolean", "symbol", "undefined", "object", "function".
Applied to files:
docs/framework/angular/typescript.md
📚 Learning: 2025-08-19T03:18:18.303Z
Learnt from: oscartbeaumont
Repo: TanStack/query PR: 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:
docs/framework/angular/guides/query-invalidation.mdpackages/angular-query-experimental/src/inject-queries.tspackages/angular-query-experimental/src/inject-query.tspackages/angular-query-experimental/src/query-options.tspackages/angular-query-experimental/src/types.ts
🧬 Code graph analysis (18)
packages/angular-query-experimental/src/__tests__/pending-tasks-ssr.test.ts (1)
packages/angular-query-experimental/src/inject-query.ts (1)
injectQuery(224-232)
packages/angular-query-experimental/src/__tests__/inject-infinite-query.test-d.ts (2)
packages/angular-query-experimental/src/inject-infinite-query.ts (1)
injectInfiniteQuery(115-140)packages/query-core/src/types.ts (1)
InfiniteData(204-207)
packages/angular-query-experimental/src/__tests__/inject-devtools-panel.test.ts (1)
packages/angular-query-experimental/src/__tests__/test-utils.ts (1)
setupTanStackQueryTestBed(52-64)
packages/angular-query-experimental/src/__tests__/test-utils.ts (1)
packages/query-core/src/queryObserver.ts (1)
options(379-385)
packages/angular-query-experimental/src/__tests__/inject-queries.test-d.ts (1)
packages/angular-query-experimental/src/inject-queries.ts (1)
injectQueries(271-381)
packages/angular-query-experimental/src/__tests__/with-devtools.test.ts (1)
packages/angular-query-experimental/src/__tests__/test-utils.ts (1)
flushQueryUpdates(70-72)
packages/angular-query-experimental/src/__tests__/signal-proxy.test.ts (2)
packages/angular-query-experimental/src/signal-proxy.ts (1)
signalProxy(19-61)packages/angular-query-experimental/src/__tests__/test-utils.ts (1)
registerSignalInput(83-100)
packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts (1)
packages/angular-query-experimental/src/__tests__/test-utils.ts (1)
registerSignalInput(83-100)
packages/angular-query-experimental/src/inject-infinite-query.ts (3)
packages/query-core/src/types.ts (3)
DefaultError(47-51)InfiniteData(204-207)QueryKey(53-61)packages/angular-query-experimental/src/types.ts (1)
CreateInfiniteQueryOptions(69-84)packages/angular-query-experimental/src/signal-proxy.ts (1)
MethodKeys(4-6)
packages/angular-query-experimental/src/__tests__/inject-is-fetching.test.ts (1)
packages/angular-query-experimental/src/__tests__/test-utils.ts (1)
setupTanStackQueryTestBed(52-64)
packages/angular-query-experimental/src/__tests__/inject-is-mutating.test.ts (1)
packages/angular-query-experimental/src/__tests__/test-utils.ts (2)
setupTanStackQueryTestBed(52-64)flushQueryUpdates(70-72)
packages/angular-query-experimental/src/__tests__/mutation-options.test.ts (1)
packages/angular-query-experimental/src/__tests__/test-utils.ts (2)
setupTanStackQueryTestBed(52-64)flushQueryUpdates(70-72)
packages/angular-query-experimental/src/__tests__/inject-mutation-state.test.ts (1)
packages/angular-query-experimental/src/__tests__/test-utils.ts (1)
registerSignalInput(83-100)
packages/angular-query-experimental/src/__tests__/inject-queries.test.ts (2)
packages/angular-query-experimental/src/__tests__/test-utils.ts (1)
setupTanStackQueryTestBed(52-64)packages/angular-query-experimental/src/inject-queries.ts (1)
injectQueries(271-381)
packages/angular-query-experimental/src/__tests__/inject-infinite-query.test.ts (3)
packages/angular-query-experimental/src/__tests__/test-utils.ts (1)
setupTanStackQueryTestBed(52-64)examples/angular/infinite-query-with-max-pages/src/app/components/example.component.ts (1)
Component(12-54)packages/angular-query-experimental/src/inject-infinite-query.ts (1)
injectInfiniteQuery(115-140)
packages/angular-query-experimental/src/__tests__/pending-tasks.test.ts (1)
packages/angular-query-experimental/src/__tests__/test-utils.ts (2)
setupTanStackQueryTestBed(52-64)flushQueryUpdates(70-72)
packages/angular-query-experimental/src/query-options.ts (1)
packages/angular-query-experimental/src/types.ts (1)
CreateQueryOptions(29-37)
packages/angular-query-experimental/src/types.ts (2)
packages/svelte-query/src/types.ts (3)
CreateBaseQueryOptions(24-30)CreateQueryOptions(39-44)CreateInfiniteQueryResult(68-71)packages/angular-query-experimental/src/signal-proxy.ts (2)
MapToSignals(8-10)MethodKeys(4-6)
🪛 markdownlint-cli2 (0.18.1)
docs/framework/angular/installation.md
10-10: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (96)
.changeset/deep-crews-open.md (1)
2-2: Verify the version level: should this be "major" instead of "minor"?The PR objectives note that Angular <19 support was removed and Angular v19+ is now required. This is a breaking change for consumers using Angular <19, which typically warrants a
majorversion bump rather thanminor.Confirm whether this changeset should be updated to reflect the breaking nature of the change.
docs/framework/angular/guides/infinite-queries.md (1)
22-22: Approve documentation examples for Angular v19 DI patterns.The additions of
projectsService = inject(ProjectsService)consistently demonstrate Angular's dependency-injection pattern in both examples. The queryFn correctly capturesthisvia arrow function closure to access the injected service. This aligns well with the PR objective to correct examples and include properthis.references, making the documentation clearer for users implementing infinite queries with Angular v19.Also applies to: 78-79
docs/framework/angular/guides/caching.md (1)
30-30: Typo correction approved.The correction from "que yFn" to "queryFn" improves clarity in the documentation narrative.
examples/angular/infinite-query-with-max-pages/src/app/components/example.component.ts (1)
32-53: Nice simplification—direct query API access eliminates duplication.The refactor from private state mirrors to direct query method calls (
hasNextPage(),isFetchingNextPage(), etc.) is clean and correct. All methods are properly invoked as signals withincomputed(), establishing the right reactive dependencies. This aligns well with the signal-based improvements in Angular 19.docs/framework/angular/guides/testing.md (2)
32-32: Clear guidance on test setup patterns.The expansion of the
afterEachsection effectively communicates the rationale for preferring a freshQueryClientper test over reusing and clearing, emphasizing explicit setup over "invisible globals." This aligns well with the PR's emphasis on deterministic, lifecycle-aware testing.
47-47: Add version-specific guidance for effect triggering in Angular 19 vs. 20+.Examples at lines 47 and 140 use
TestBed.tick()to trigger effects, but this API was introduced in Angular 20. Since this documentation targets Angular 19+, provide guidance for Angular 19 users (e.g., useTestBed.flushEffects()orTestBed.inject(ApplicationRef).tick()) to ensure examples work across supported versions.docs/framework/angular/guides/disabling-queries.md (2)
16-21: Template syntax leverages Angular 19 control flow with proper signal binding.The change from direct signal calls inside
@forto bindingquery.data()once via@if (query.data(); as data)is a best practice for signal-based components. This avoids redundant signal reads and improves change detection efficiency.
73-73: Import path aligns with Angular Query experimental package migration.The @tanstack/angular-query-experimental package offers a 1st-class API for using TanStack Query via Angular. All Angular guide examples consistently import injectQuery and queryOptions from @tanstack/angular-query-experimental. This import source is used uniformly across all Angular documentation examples, guides, and quick-start materials.
packages/angular-query-experimental/package.json (2)
104-105: Test coverage aligns correctly with Angular 19 requirements.The peerDependencies now require Angular >=19.0.0, and test scripts only cover TypeScript 5.5+. This is correct: Angular 19 dropped support for TypeScript versions below 5.5, so the testing strategy appropriately covers the supported range.
91-91: @angular/platform-server belongs in devDependencies.The package is correctly placed as a development dependency since it's used only for SSR testing with PendingTasks integration, not in runtime code. The library's runtime exports use @angular/core, HttpClient, and @angular/common/http, not @angular/platform-server.
docs/framework/angular/reference/functions/injectQuery.md (1)
12-21: LGTM! Consistent use oflastValueFromfor Observable-to-Promise conversion.The examples correctly demonstrate wrapping Angular's HttpClient observables with
lastValueFromto return a Promise, which aligns with TanStack Query's expectedqueryFnreturn type.docs/framework/angular/guides/paginated-queries.md (1)
38-41: Good migration to boolean signal methods.Using
query.isPending()andquery.isError()instead of status-based checks aligns with the updated TypeScript narrowing guidance and improves type inference in Angular templates.docs/framework/angular/typescript.md (2)
95-97: Clear and accurate guidance on TypeScript narrowing limitations.The explanation correctly notes that TypeScript doesn't support discriminated unions on object methods, and the recommendation to use boolean signal methods (
isSuccess(),isError(),isPending()) is the right workaround for Angular's signal-based architecture.
155-171: Improved defaultError example with practical AxiosError usage.Using
AxiosErrorinstead ofunknownas thedefaultErrortype provides a more realistic and useful example of module augmentation for error typing.docs/framework/angular/guides/dependent-queries.md (1)
12-23: Good update to use instance-bound methods.Using
this.email(),this.getUserByEmail, andthis.getProjectsByUsercorrectly demonstrates class-based Angular component patterns and is consistent with the rest of the documentation updates.packages/angular-query-experimental/src/inject-infinite-query.ts (2)
142-146: LGTM! Correctly excludes action methods from signal conversion.The
methodsToExcludearray properly identifiesfetchNextPage,fetchPreviousPage, andrefetchas methods that should remain as regular functions rather than being wrapped in signals. This prevents early signal reads and maintains the expected imperative API for these action methods.
115-139: Clean integration with the signal proxy exclusion mechanism.The implementation properly passes
methodsToExcludetocreateBaseQuery, maintaining consistency with theinjectQueryimplementation. The generic type parameters are well-structured with appropriate defaults.packages/angular-query-experimental/src/signal-proxy.ts (3)
4-10: Well-designed type utilities for selective signal exclusion.The
MethodKeysutility correctly extracts function property names, and the updatedMapToSignalstype conditionally preserves original function types for excluded fields while converting others to signals. This provides accurate type inference for consumers.
34-41: Correct use ofuntrackedfor excluded methods.Wrapping the function call in
untracked(inputSignal)[prop]ensures that calling these methods (likerefetch) doesn't create a reactive dependency on the input signal. This prevents the issue of reading input signals too early during component initialization, as mentioned in the PR objectives.
47-58: Consider usinguntrackedconsistently inownKeyshandler.The
hashandler at line 48 andownKeyshandler at line 51 correctly useuntrackedto avoid creating reactive dependencies. This is consistent and appropriate for these metadata operations.docs/framework/angular/guides/queries.md (1)
22-24: LGTM! Good documentation improvement.Adding the
@Componentdecorator provides better context for the example, making it clearer thatTodosComponentis a complete Angular component.packages/angular-query-experimental/README.md (2)
81-84: LGTM! Complete NgModule example.Adding
provideHttpClient()and theAppModuleexport makes this a more complete, copy-pasteable example. TheprovideHttpClient()addition is particularly helpful since many users will need HTTP client for their query functions.
32-32: Clarify Angular version requirement discrepancy.The README correctly reflects the package.json peer dependencies (Angular 19+). However, official TanStack Query documentation states the adapter requires Angular 16 or higher. Verify this upgrade from 16+ to 19+ is intentional and update the package.json comment or documentation if needed to explain the minimum requirement.
packages/angular-query-experimental/src/__tests__/with-devtools.test.ts (1)
16-16: LGTM! Test utility import.Importing the centralized
flushQueryUpdatesutility is consistent with the test refactoring pattern used across the test suite.docs/framework/angular/guides/does-this-replace-client-state.md (1)
10-11: LGTM! Angular-specific terminology.Replacing React-centric state management references (Redux, MobX, Zustand) with Angular equivalents (NgRx Store, RxJS) makes the documentation more relevant and relatable for Angular developers.
packages/angular-query-experimental/src/__tests__/inject-devtools-panel.test.ts (1)
1-6: LGTM! Centralized test utility imports.Switching to
setupTanStackQueryTestBedreduces boilerplate and improves consistency across the test suite.docs/framework/angular/overview.md (2)
16-16: LGTM! Angular 19+ requirement.The version requirement update is consistent across all documentation files in this PR and aligns with the package's new minimum Angular version.
76-80: LGTM! Modern Angular Query patterns.The updated template demonstrates best practices:
- Using
isError()andisSuccess()status methods for better type discrimination- Leveraging Angular 19's
@letsyntax for local template variables- Improved type narrowing by checking status before accessing data
These changes provide a better example for users to follow.
packages/angular-query-experimental/src/__tests__/inject-infinite-query.test-d.ts (1)
1-96: LGTM! Comprehensive type coverage for discriminated unions.The new type tests thoroughly validate:
- Default union types for
dataanderrorsignals- Type narrowing when checking
isSuccess(),isPending(), andisError()- Correct typing for the
selectfunction parameter and return typeThis provides strong type safety guarantees and aligns with the discriminated union improvements mentioned in the PR objectives.
packages/angular-query-experimental/src/__tests__/inject-query.test-d.ts (1)
36-46: LGTM! Select function type coverage.The new test validates that:
- The
selectfunction receives the correct input type (string from queryFn)- The returned data signal correctly reflects the transformed type (
Signal<number | undefined>)This strengthens type safety guarantees for the select transformation feature.
packages/angular-query-experimental/src/__tests__/inject-queries.test-d.ts (1)
179-262: LGTM! Comprehensive type tests for the combine function.These tests thoroughly cover the typing scenarios for
injectQuerieswith thecombineoption:
- Validates correct data types in the combine callback
- Confirms
refetchis callable (aligned with themethodsToExcludepattern)- Tests branded result types with
Signalwrapper- Verifies
initialDatacorrectly removesundefinedfrom the data typedocs/framework/angular/guides/initial-query-data.md (2)
84-91: LGTM! Correctly updated to component-scoped patterns.The changes properly add
this.prefix for component-scoped access toqueryClientand use dynamictodoId()in the fetch path, aligning with Angular component conventions throughout the documentation.
97-108: Verify queryKey consistency across examples.Line 99 uses
queryKey: ['todos', this.todoId()]while line 83 usesqueryKey: ['todo', this.todoId()](singular vs plural). This may be intentional to demonstrate different patterns, but verify this is the intended behavior.docs/framework/angular/guides/mutations.md (1)
246-256: LGTM! Improved component example with proper Angular conventions.The changes correctly:
- Add
@Componentdecorator making it a complete Angular component example- Rename class to
SomeComponentfollowing PascalCase conventions- Use
this.mutation.mutate()for proper component-scoped accesspackages/angular-query-experimental/src/__tests__/pending-tasks-ssr.test.ts (2)
20-28: LGTM! Proper test lifecycle setup.Good use of
destroyPlatform()for cleanup between tests and fake timers setup/teardown. This ensures test isolation for SSR scenarios.
46-71: Well-structured SSR integration test.The test correctly:
- Bootstraps with zoneless change detection and server rendering
- Configures QueryClient with
retry: falseto avoid flakiness- Uses
vi.runAllTimersAsync()before awaiting the HTML promise to properly advance fake timers- Verifies the query data appears in the server-rendered output
This validates that Angular's
PendingTasksintegration works correctly with TanStack Query during SSR.docs/framework/angular/guides/invalidations-from-mutations.md (1)
5-10: LGTM!The documentation updates appropriately align the example with Angular conventions by adding the
@Componentdecorator and expanding the replacement mapping for better React-to-Angular translation.Also applies to: 30-33
docs/framework/angular/guides/query-invalidation.md (1)
11-16: LGTM!The addition of the
@Componentdecorator and proper Angular imports makes the example more representative of real-world Angular usage.packages/angular-query-experimental/src/__tests__/inject-is-mutating.test.ts (1)
3-6: LGTM!The migration to shared test utilities (
setupTanStackQueryTestBedandflushQueryUpdates) appropriately centralizes test setup logic and improves maintainability.Also applies to: 15-15, 38-38
packages/angular-query-experimental/src/__tests__/inject-is-fetching.test.ts (1)
3-6: LGTM!Consistent migration to the shared test utility pattern, maintaining test behavior while improving code organization.
Also applies to: 15-15
docs/framework/angular/guides/query-options.md (1)
34-34: LGTM!The added example effectively demonstrates how to use
QueryClientmethods (prefetchQueryandsetQueryData) withqueryOptions, providing helpful guidance for readers.Also applies to: 36-43
docs/framework/angular/guides/query-retries.md (1)
33-37: LGTM!The documentation correctly updates to use
provideTanStackQueryandbootstrapApplication, aligning with the Angular Query integration pattern.Also applies to: 54-54
packages/angular-query-experimental/src/__tests__/mutation-options.test.ts (1)
11-11: LGTM!Consistent application of the test utilities pattern across all test cases, replacing direct timer manipulation with the abstracted
flushQueryUpdates()helper.Also applies to: 19-19, 58-58, 78-78, 106-106, 134-134
packages/angular-query-experimental/src/__tests__/inject-infinite-query.test.ts (1)
22-67: LGTM!The component-based testing approach with
ChangeDetectionStrategy.OnPushprovides more realistic test coverage and aligns well with the PR's goal of testing Angular Query in component contexts.docs/framework/angular/guides/query-functions.md (1)
12-84: LGTM! Documentation correctly demonstrates signal-based query patterns.The examples consistently show signal function call syntax (e.g.,
todoId(),this.todoId()) for reactive query keys and query functions, which aligns with Angular's signal-based reactivity model and the broader changes in this PR.packages/angular-query-experimental/src/__tests__/inject-queries.test.ts (3)
19-25: LGTM! Test setup properly uses fake timers.The addition of
vi.useFakeTimers({ shouldAdvanceTime: true })in beforeEach andvi.useRealTimers()in afterEach ensures proper test isolation and timer control for async operations.
83-158: Excellent test coverage for the combine function.This test thoroughly validates:
- Combining multiple query results into a custom shape
- The refetch method from the combined result
- Progressive data updates (empty → partial → complete)
- 5 distinct snapshots showing data progression through refetch cycles
The test correctly verifies both the combined data string format and the refetch functionality.
160-234: Comprehensive test for dynamic queries array.This test validates important reactive behaviors:
- Dynamic addition/removal of queries via signal updates
- Correct data mapping and aggregation
- Multiple change detection cycles (7 and 9 snapshots respectively)
- Edge case of empty queries array
The test correctly exercises the reactive nature of the queries array and verifies that the UI updates appropriately.
packages/angular-query-experimental/src/query-options.ts (1)
44-48: LGTM! Type simplification enables select function support.Removing the
Omit<..., 'queryFn'>wrapper and usingCreateQueryOptionsdirectly reintroduces thequeryFnproperty intoDefinedInitialDataOptions. This change aligns with other signal-based adapters and fixes the select function on queries as noted in the PR objectives.docs/framework/angular/guides/default-query-function.md (1)
31-54: LGTM! Documentation correctly demonstrates component-based query usage.The addition of
@Componentdecorators and the use of Angular 19'sinput()signal pattern make the examples more complete and realistic. The transition frompostIdSignal()topostId()is consistent with the signal input API.packages/angular-query-experimental/src/__tests__/inject-mutation-state.test.ts (1)
161-181: LGTM! Host component pattern properly tests signal inputs.The introduction of
HostComponentwithregisterSignalInputcorrectly enables testing of components with required signal inputs in Angular's new testing paradigm. The use ofnativeElement.querySelectorAllfor DOM queries is appropriate for this test scenario.packages/angular-query-experimental/src/__tests__/signal-proxy.test.ts (2)
18-18: Note the exclusion mechanism.The signalProxy now accepts a second argument
['fn']to specify which fields should NOT be converted to signals. This is a key change enabling the refactored proxy behavior described in the PR objectives.
63-95: Excellent component fixture tests for signal proxy behavior.These tests thoroughly validate:
- Fields are generated lazily after first change detection
- Excluded fields (fn) remain functions while others become signals
- Short accessors correctly reference the proxy fields
- Proxy fields reactively update when the underlying signal changes
The tests properly exercise the signal proxy in a realistic Angular component context with OnPush change detection.
packages/angular-query-experimental/src/inject-query.ts (1)
230-234: LGTM! Method exclusion mechanism correctly implemented.The introduction of
methodsToExcludewith['refetch']ensures that the refetch method remains a function rather than being converted to a signal. This prevents issues with reading input signals too early (as mentioned in PR objectives) and maintains the expected API surface whererefetchis called as a function.The type safety via
Array<MethodKeys<QueryObserverResult>>ensures only valid method names can be excluded.docs/framework/angular/guides/parallel-queries.md (1)
20-62: LGTM! Documentation examples correctly demonstrate parallel queries.The addition of
@Componentdecorators makes the examples more complete and realistic. The experimental notice forinjectQueriesappropriately sets user expectations.packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts (3)
1-15: LGTM!Import updates are appropriate:
ChangeDetectionStrategyfor OnPush components,firstValueFromfor RxJS observable handling, andregisterSignalInputfrom test utilities.
302-354: LGTM!The host-wrapper pattern for testing signal inputs is well-structured. Using
registerSignalInputto mark the input as signal-based before creating the fixture ensures proper Angular change detection behavior.
661-704: LGTM!The optimistic updates test properly validates the pending tasks lifecycle:
- Verifies optimistic update applies synchronously
- Confirms
app.isStableisfalsewhile mutation is pending- Verifies completion after
whenStable()resolvesThis effectively tests the PendingTasks integration for SSR and stability tracking.
packages/angular-query-experimental/src/__tests__/test-utils.ts (3)
48-64: LGTM!
setupTanStackQueryTestBedprovides a clean abstraction for test setup, properly accepting optional additional providers viaEnvironmentProviders | Providerunion.
66-72: LGTM!Clear documentation explaining why advancing timers by 0 is needed for TanStack Query's notifyManager updates.
76-99: LGTM with note on internal API usage.The
registerSignalInputfunction correctly documents its temporary nature using the theta (ɵ) prefix, indicating Angular's experimental APIs. The error handling for missing component definitions is appropriate. The ɵcmp and ɵdir fields are standard component/directive definitions generated by Angular's Ivy compiler, and while internal, they remain valid in Angular 19. The comment about Angular 21 CLI supporting Vitest compilation is accurate—Vitest is now promoted to stable as the default test runner in Angular v21.docs/framework/angular/guides/placeholder-query-data.md (3)
10-19: LGTM!The example now properly shows the Angular component wrapper around the
injectQueryusage.
27-40: LGTM!Good example demonstrating the reactive pattern with
signal()fortodoId, showing how bothqueryKeyandqueryFncan depend on the signal value.
45-65: LGTM!Good example showing
input.required()usage with queries and how to access cached data fromQueryClientfor placeholder data.packages/angular-query-experimental/src/inject-mutation.ts (4)
1-11: LGTM!Import change from custom
PENDING_TASKSto Angular's nativePendingTasksis the right approach for Angular 19+ compatibility.
84-98: LGTM!The pending task lifecycle helpers are well-guarded:
startPendingTaskprevents double-start with!taskCleanupRef && !destroyedstopPendingTaskproperly nullifies the ref after invoking cleanup
141-181: LGTM!The subscriber callback properly:
- Guards against destroyed state before processing
- Tracks pending tasks based on
isPendingstate- Cleans up in correct order: set
destroyed=true, stop pending task, then unsubscribe
196-200: LGTM!The
excludeFunctionsparameter correctly lists the imperative methods (mutate,mutateAsync,reset) that should remain callable functions rather than being wrapped as signals.packages/angular-query-experimental/src/create-base-query.ts (7)
1-25: LGTM!Import updates properly bring in Angular's native APIs (
DestroyRef,PendingTasks,linkedSignal) and the necessary type imports for the refactored implementation.
86-93: LGTM!The lazy initialization pattern is well-designed:
computed()creates the observer only when first accesseduntracked(defaultedOptionsSignal)prevents the computed from tracking option changes- The separate effect handles option synchronization after creation
95-127: LGTM!The result tracking implementation correctly integrates with TanStack Query's property tracking optimization. The
void valuepattern (line 125) idiomatically signals that the access is intentional for tracking purposes.
180-194: LGTM!The dual cleanup strategy is robust:
- Effect's
onCleanuphandles normal effect lifecycle cleanupdestroyRef.onDestroyensures cleanup even if effect cleanup doesn't run (edge cases)The
isRestoring()guard correctly prevents subscription during hydration/restore scenarios.
196-199: LGTM!Using
resultSignal.asReadonly()properly encapsulates the signal state, and theexcludeFunctionsparameter correctly propagates tosignalProxy.
169-178:linkedSignalis available in Angular 19 (experimental) and became stable in Angular 20.LinkedSignal was introduced in Angular Version 19, though it was promoted to stable in Angular 20. The code using
linkedSignalis compatible with Angular 19 but will have experimental status at that version level.
129-167: The destroyed check timing is correct and already verified by tests.The
destroyedcheck at line 144 prevents stale updates by executing inside the queued microtask before state modifications. Test coverage inpending-tasks.test.tsexplicitly validates this behavior with dedicated component destruction tests that confirm pending tasks are cleaned up and Angular stability is restored when components with active queries or mutations are destroyed.packages/angular-query-experimental/src/__tests__/inject-query.test.ts (7)
1-31: LGTM! Imports properly updated for component-based testing.The imports now include
ChangeDetectionStrategy,NgZone, andregisterSignalInputto support the new component-wrapped test approach with OnPush change detection.
53-260: Well-structured type inference test with comprehensive coverage.The refactoring to a component-based approach allows all type inference tests to run within a proper Angular injection context. The tests cover:
- Unspecified queryFn defaulting to unknown
- Explicit generic type parameters
- Union types, wrapped queries, and various queryFn signatures
578-619: Proper error handling cleanup for uncaughtException listener.The error handling tests correctly manage the
process.on('uncaughtException')listener lifecycle with a try/finally block ensuring cleanup even if the test fails. This prevents listener leaks across tests.
726-747: Good test for effect ordering with query injection.This test validates an important edge case: effects registered before
injectQueryshould be able to safely read query data without throwing. The spy correctly verifies the initial undefined state.
749-785: Excellent test for synchronous input availability.This test validates that when
setInputis called before change detection, the query can immediately access the input signal value and return cached data. This is important for avoiding unnecessary pending states with pre-populated cache.
827-845: Good test for same-macrotask data updates.Uses
TestBed.tick()to trigger effects (valid Angular 20+ API) and validates that query data updates are reflected within the same macrotask after flushing microtasks. Based on learnings,TestBed.tick()is the appropriate API for triggering effects in unit tests.
884-914: Validates PendingTasks integration correctly.This test ensures that
whenStable()waits for query completion via the PendingTasks integration. The pattern of advancing timers and then awaitingwhenStable()correctly tests the async coordination.packages/angular-query-experimental/src/inject-queries.ts (4)
95-111: Clean generic wrapper for initialData type resolution.
GenericGetDefinedOrUndefinedQueryResultelegantly abstracts the initialData logic to work with any result type pair, eliminating code duplication betweenGetCreateQueryResultandGetQueryObserverResult.
113-143: Well-structured type inference for data and error types.
InferDataAndErrorconsolidates the type inference logic shared between signal-wrapped results and raw observer results, following the same three-part pattern (object form, tuple form, queryFn inference) used inGetCreateQueryOptionsForCreateQueries.
236-264: Appropriate type change for combine callback.
RawQueriesResultscorrectly maps to plainQueryObserverResulttypes for the combine function, as the combine callback receives raw observer results rather than signal-proxied values. This aligns with how other adapters (Vue) handle the combine function.
374-379: Thepromiseproperty needs special handling, but cannot be excluded via the existing mechanism.
QueryObserverResultdoes have apromiseproperty (typePromise<TData>) that is currently wrapped asSignal<Promise<TData>>bysignalProxy. However, it cannot be added to the['refetch']exclusion list becausesignalProxyonly accepts function properties in itsexcludeFieldsparameter—the typeMethodKeys<T>explicitly filters to functions only.This creates an issue: the
promiseproperty violates TanStack Query's "stable promise" contract (documented as requiringexperimental_prefetchInRenderand meant forReact.use()), since wrapping it as a signal makes it reactive rather than stable. Users must call.promise()to access the wrapped signal, breaking compatibility with the documented usage pattern.Resolving this requires expanding
signalProxyto support excluding non-function properties, or implementing alternative handling for Promise-type properties specifically.packages/angular-query-experimental/src/__tests__/pending-tasks.test.ts (5)
15-42: Good test setup with centralized configuration.Using
setupTanStackQueryTestBedand resettingonlineManager.setOnline(true)in afterEach ensures consistent test isolation. The helper reduces boilerplate and ensures consistent module configuration across tests.
219-265: Excellent test for offline-start edge case.This test validates a subtle but important scenario: when a query initializes while offline with
networkMode: 'online', it goes directly topausedwithout ever reachingfetching. The test correctly verifies that PendingTasks still blocks stability until the app comes back online and the query completes.
400-448: Well-structured concurrent queries test.Using a single component with multiple query properties provides clean access via
fixture.componentInstanceand properly tests that all concurrent queries complete beforewhenStable()resolves.
537-542: Clean provider composition with setupTanStackQueryTestBed.The
providersoption correctly layers HttpClient testing providers on top of the base TanStack Query configuration, maintaining separation of concerns.
356-392: fixture.isStable() appropriately checks fixture-level pending tasks.The destruction tests correctly use
fixture.isStable()andfixture.whenStable()to verify pending tasks cleanup. ComponentFixture.isStable() returns !this.pendingTasks.hasPendingTasks, and TanStack Query's inject* functions integrate with PendingTasks which ensures the framework is aware of in-progress queries and mutations. This pattern is explicitly documented as the recommended approach for testing query/mutation completion.packages/angular-query-experimental/src/types.ts (5)
19-19: Appropriate import addition for MethodKeys.Importing
MethodKeysalongsideMapToSignalsenables the type system to correctly identify and exclude method properties from signal conversion.
21-37: Simplified type aliases align with Svelte adapter pattern.Changing
CreateBaseQueryOptionsandCreateQueryOptionsfrom interfaces to type aliases matches the pattern used inpackages/svelte-query/src/types.ts(as shown in the relevant code snippets), promoting consistency across framework adapters.
86-94: Correct MapToSignals usage with MethodKeys.The pattern
MapToSignals<T, MethodKeys<T>>ensures that method properties (likerefetch,promise) remain as callable functions rather than being wrapped in signals. TheOmitKeyof<TState, keyof BaseQueryNarrowing, 'safely'>correctly excludes the narrowing predicates before applying the transformation.
266-282: Mutation result type correctly updated.
CreateMutationResultproperly appliesMapToSignalswithMethodKeyswhile excludingBaseMutationNarrowingpredicates, maintaining consistency with the query result types.
111-130: Consistent MethodKeys pattern for infinite query results.Both
CreateInfiniteQueryResultandDefinedCreateInfiniteQueryResultcorrectly apply theMethodKeyspattern. Note thatDefinedCreateInfiniteQueryResultdoesn't includeBaseQueryNarrowingin its definition, which differs from the non-infinite variants.Verify this is intentional - the defined infinite query result type does not include the
isSuccess/isError/isPendingnarrowing predicates:#!/bin/bash # Check if DefinedCreateInfiniteQueryResult should include BaseQueryNarrowing rg -n "DefinedCreateInfiniteQueryResult" --type=ts packages/angular-query-experimental/src/
| userIds = injectQuery(() => ({ | ||
| queryKey: ['users'], | ||
| queryFn: getUserData, | ||
| select: (users) => users.map((user) => user.id), | ||
| })) | ||
|
|
||
| // Then get the users messages | ||
| userQueries = injectQueries(() => ({ | ||
| queries: (this.userIds() ?? []).map((userId) => ({ | ||
| queryKey: ['user', userId], | ||
| queryFn: () => getUserById(userId), | ||
| })), | ||
| })) |
There was a problem hiding this comment.
Incorrect data access pattern in injectQueries example.
Line 43 uses this.userIds() but userIds is the query result object from injectQuery, not a signal of the data. To access the selected user IDs, it should be this.userIds.data().
🔎 Proposed fix
// Then get the users messages
userQueries = injectQueries(() => ({
- queries: (this.userIds() ?? []).map((userId) => ({
+ queries: (this.userIds.data() ?? []).map((userId) => ({
queryKey: ['user', userId],
queryFn: () => getUserById(userId),
})),
}))📝 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.
| userIds = injectQuery(() => ({ | |
| queryKey: ['users'], | |
| queryFn: getUserData, | |
| select: (users) => users.map((user) => user.id), | |
| })) | |
| // Then get the users messages | |
| userQueries = injectQueries(() => ({ | |
| queries: (this.userIds() ?? []).map((userId) => ({ | |
| queryKey: ['user', userId], | |
| queryFn: () => getUserById(userId), | |
| })), | |
| })) | |
| userIds = injectQuery(() => ({ | |
| queryKey: ['users'], | |
| queryFn: getUserData, | |
| select: (users) => users.map((user) => user.id), | |
| })) | |
| // Then get the users messages | |
| userQueries = injectQueries(() => ({ | |
| queries: (this.userIds.data() ?? []).map((userId) => ({ | |
| queryKey: ['user', userId], | |
| queryFn: () => getUserById(userId), | |
| })), | |
| })) |
🤖 Prompt for AI Agents
In docs/framework/angular/guides/dependent-queries.md around lines 35 to 47, the
injectQueries example incorrectly calls this.userIds() (userIds is a query
result object), so update the code to read the selected IDs from the query
result by using this.userIds.data() when mapping; ensure you also keep a safe
fallback like (this.userIds.data() ?? []) so the map runs on an array.
| ### NPM | ||
|
|
||
| _Angular Query is compatible with Angular v16 and higher_ | ||
| _Angular Query is compatible with Angular v19 and higher_ |
There was a problem hiding this comment.
Use blockquote format instead of emphasis for the compatibility note.
The compatibility update to Angular v19+ is correct and aligns with the PR objectives. However, the current emphasis formatting using underscores triggers a markdown linting rule (MD036) that discourages using emphasis as a heading/callout. Use a blockquote format instead for better semantic clarity and linting compliance.
🔎 Proposed fix using blockquote format
-_Angular Query is compatible with Angular v19 and higher_
+> Angular Query is compatible with Angular v19 and higher📝 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.
| _Angular Query is compatible with Angular v19 and higher_ | |
| > Angular Query is compatible with Angular v19 and higher |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
10-10: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In docs/framework/angular/installation.md at line 10, the compatibility note
currently uses emphasis underscores ("_Angular Query is compatible with Angular
v19 and higher_") which triggers MD036; replace that emphasis with a markdown
blockquote (prefix the line with "> ") so the note becomes a proper callout and
satisfies markdown linting.
| <button (click)="addItem()">Create</button> | ||
| <ul> | ||
| @for (task of tasks.data(); track task) { | ||
| @for (task of tasks.data(); track $index) { |
There was a problem hiding this comment.
Revert to tracking by task value for correct optimistic update behavior.
Tracking by $index instead of the task value can cause incorrect DOM updates during optimistic updates and rollbacks. When an optimistically added item is rolled back, Angular may incorrectly reuse DOM nodes because it tracks by position rather than identity, potentially displaying stale or wrong data.
For dynamic lists with add/remove operations—especially in an optimistic updates example—tracking by a stable identifier or the value itself ensures correct change detection.
🔎 Proposed fix
- @for (task of tasks.data(); track $index) {
+ @for (task of tasks.data(); track task) {📝 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.
| @for (task of tasks.data(); track $index) { | |
| @for (task of tasks.data(); track task) { |
🤖 Prompt for AI Agents
In
examples/angular/optimistic-updates/src/app/components/optimistic-updates.component.ts
around line 39, the ng-for/loop is tracking list items by $index which breaks
optimistic add/remove/rollback behavior; update the loop to track by the task
identity (preferably a stable id like task.id, or the task value if no id
exists) so Angular/Svelte will use item identity instead of position when
diffing and DOM updates will reflect correct optimistic rollbacks.
| }, | ||
| ) | ||
|
|
||
| TestBed.tick() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat packages/angular-query-experimental/package.json | jq '.peerDependencies."@angular/core"'Repository: TanStack/query
Length of output: 68
TestBed.tick() requires Angular 20, but peer dependency allows Angular 19.
TestBed.tick() was introduced in Angular 20, but the package declares @angular/core >= 19.0.0 as a peer dependency. Using TestBed.tick() will fail on Angular 19. Either update the peer dependency to require Angular 20+ or switch to an alternative approach compatible with Angular 19 (e.g., TestBed.flushEffects() for Angular 19, with a version check if needed).
🤖 Prompt for AI Agents
In
packages/angular-query-experimental/src/__tests__/inject-infinite-query.test.ts
around line 99, the test calls TestBed.tick() which is only available in Angular
20+, but the package peer dependency allows Angular 19; replace the
TestBed.tick() call with a compatible alternative or gate it behind a
runtime/version check: either (A) bump the peer dependency to require
@angular/core >=20.0.0 and keep TestBed.tick(), or (B) change the test to use
TestBed.flushEffects() (or another Angular 19-compatible API) and add a small
runtime feature-detect/version check so the test calls flushEffects() on Angular
19 and tick() on Angular 20+, ensuring the test runs across the declared peer
dependency range.
benjavicente
changed the title
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/angular-query-experimental/src/inject-query.ts (1)
234-242:⚠️ Potential issue | 🟡 Minor
@param excludeFunctionsdocuments a non-existent parameter.The implementation signature on lines 240-242 only accepts
injectQueryFnandoptions. TheexcludeFunctionsparameter described in the JSDoc doesn't exist in the public API — it's an internal detail passed tocreateBaseQuery. This is misleading for consumers reading the docs.Proposed fix
* `@param` injectQueryFn - A function that returns query options. * `@param` options - Additional configuration - * `@param` excludeFunctions - Array of function property names to exclude from signal conversion * `@returns` The query result.
🤖 Fix all issues with AI agents
In `@docs/framework/angular/reference/functions/injectQuery.md`:
- Around line 51-53: The markdown has duplicate "## Param" headings (MD024);
update the doc output for injectQuery's parameter sections by making the
headings unique or consolidated—e.g., change each "## Param" to "## Parameters"
(single consolidated section) or to distinct names like "## Param:
<paramName>"—so the three parameter blocks no longer share identical headings;
locate the injectQuery.md output (generated from JSDoc `@param` tags) and adjust
the generator/template or post-process step that renders "@param" headings to
emit a single "Parameters" header or unique headings per parameter.
In `@packages/angular-query-experimental/package.json`:
- Around line 104-107: The peerDependencies field currently allows `@angular/core`
and `@angular/common` >=19.0.0 but the code imports and uses `@angular/core` APIs
only stable in v20 (PendingTasks and linkedSignal); update package.json
peerDependencies to require ">=20.0.0" for both `@angular/core` and
`@angular/common` so consumers on Angular 19 cannot install an incompatible
version, or alternatively implement a runtime shim in the module that detects
Angular v19 and falls back to ExperimentalPendingTasks/preview linkedSignal
imports before using PendingTasks/linkedSignal; locate uses of PendingTasks and
linkedSignal in the codebase to ensure the fallback or the package.json change
covers all imports.
In `@packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts`:
- Line 721: Fix the typo in the test comment: change "waiting got the next
macrotask task" to "waiting for the next macrotask task" in the comment near the
stability check in inject-mutation.test.ts (the comment containing "waiting got
the next macrotask task").
- Line 16: Move the external rxjs import above the local/relative imports:
relocate the line importing firstValueFrom from 'rxjs' so it appears before any
local imports in inject-mutation.test.ts (i.e., ensure the import for
firstValueFrom is placed before the relative/module imports used in this test),
then run ESLint to verify the import order rule is satisfied.
In `@packages/angular-query-experimental/src/__tests__/inject-queries.test.ts`:
- Around line 14-15: Merge the two duplicate import statements from
'@tanstack/query-test-utils' into a single import that includes both symbols
(sleep and queryKey); locate the import lines that currently import sleep and
queryKey separately and replace them with one combined import that lists both
identifiers to remove the duplicate import.
In `@packages/angular-query-experimental/src/__tests__/zonejs-adapter.test.ts`:
- Around line 9-15: The import order violates the ESLint import/order rule; move
the third-party import of sleep from '@tanstack/query-test-utils' so it appears
before the relative import that brings in QueryClient, injectMutation,
injectQuery, and provideTanStackQuery (the import of '..'); reorder the two
import statements so the external module import comes first to satisfy the rule.
🧹 Nitpick comments (8)
packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts (1)
571-619: Consider extracting repeatedTestBed.resetTestingModule()+ reconfigure blocks.Several tests in the
injection contextdescribe block reset and reconfigure TestBed identically. A shared helper or a nestedbeforeEachcould reduce this duplication.packages/angular-query-experimental/src/__tests__/inject-query.test.ts (2)
59-266: Comprehensive type-checking test successfully migrated to component-based approach.The use of IIFEs for locally-scoped type declarations within the component class is a pragmatic approach. Type assertions look correct across all query variants.
Line 119 has a
// todo use query options?comment — consider creating an issue to track this if it represents a real intent.Would you like me to open an issue to track the TODO on line 119?
584-626: Error-handling test is thorough but has duplicated setup with the next test.The
NgZone.onError+process.on('uncaughtException')pattern withtry/finallycleanup is well done. The pattern is duplicated almost identically in the "throwOnError function returns true" test (lines 628-670). Consider extracting the error-capture setup into a helper if more throwOnError variants are added in the future, but this is fine for now.packages/angular-query-experimental/src/__tests__/inject-queries.test.ts (2)
118-193: Good coverage of thecombinefunction behavior.This test thoroughly validates combined results including the refetch-all pattern and snapshot progression. The effect-driven result tracking is a solid approach.
One nit: Line 151
results.forEach((r) => r.refetch())— Biome flags that the arrow callback implicitly returns therefetch()promise. Wrapping in braces suppresses this:Proposed fix
- refetch: () => results.forEach((r) => r.refetch()), + refetch: () => results.forEach((r) => { r.refetch() }),
368-442: Well-structured test for dynamic query array changes with optimistic transitions.The signal
queriesis declared at line 398, after thePageclass definition at lines 371-396 that referencesqueries(). This works because the class body closures aren't invoked until the component is instantiated (line 400), by which time the signal exists. However, for readability, consider moving the signal declaration before the class definition to match the pattern used in other tests (e.g., lines 274, 313, 445).packages/angular-query-experimental/src/inject-queries.ts (3)
418-435: Proxy recreation on every result change.
proxiedResultsSignalrecreates all proxy wrappers for everyresultSignalupdate, including when only one query's data changed. EachcreateResultProxyallocates a newcomputed+Proxyper element.For small query arrays this is fine. For larger arrays with frequent updates (e.g., polling), the allocation churn could become noticeable. An alternative would be to maintain a stable array of proxies sized to the current result length, reusing existing proxies when indices don't change — similar to how the
linkedSignalpattern preserves identity.Not urgent given typical
injectQueriesusage, but worth keeping in mind if performance reports surface.
442-451: Unnecessary reactive read ofresultSignal()on the non-combine path.
resultSignal()is read unconditionally on line 443, creating a dependency that triggers recomputation even when the non-combine branch exclusively returnsproxiedResultsSignal()(which internally readsresultSignal()on its own). This means changes to the raw result array cause this outercomputedto re-evaluate twice in the non-combine case — once from its own dependency, and once propagated throughproxiedResultsSignal.Consider reading
resultSignal()only inside thecombinebranch:Proposed fix
return computed(() => { - const result = resultSignal() const { combine } = optionsSignal() if (combine) { - return result + return resultSignal() } return proxiedResultsSignal() as unknown as TCombinedResult })
376-416: Stale microtask can briefly set an outdated combined result.When the effect re-runs (e.g., because
defaultedQuerieschanged), the timeline is:
- Old subscription callback fires → queues a microtask with the old
getCombinedResultonCleanupruns synchronously → unsubscribes (no future callbacks)- Effect body re-runs → creates a new subscription
- The already-queued microtask from step 1 executes →
resultSignal.set(getCombinedResult(state))with the stalegetCombinedResultThe stale value will be overwritten on the next notification from the new subscription, so the window is very short. Adding an
effectRef-style generation counter that the microtask checks before writing would close the gap:Sketch
+ let effectGeneration = 0 + effect((onCleanup) => { const observer = observerSignal() const [optimisticResult, getCombinedResult] = optimisticResultSignal() + const currentGen = ++effectGeneration ... queueMicrotask(() => { - if (destroyed) return + if (destroyed || currentGen !== effectGeneration) return ... }) ... })This is a narrow race; feel free to defer if you consider the transient stale value acceptable.
| "peerDependencies": { | ||
| "@angular/common": ">=16.0.0", | ||
| "@angular/core": ">=16.0.0" | ||
| "@angular/common": ">=19.0.0", | ||
| "@angular/core": ">=19.0.0" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are any Angular version-specific imports or APIs that might not exist in Angular 19
# Look for imports from `@angular/core` that might be v20-only
echo "=== Checking `@angular/core` imports ==="
rg -n "from '@angular/core'" --type ts -g '!node_modules/**' packages/angular-query-experimental/src/ 2>/dev/null | head -30
echo -e "\n=== Checking for Angular v20-specific APIs ==="
rg -n "pendingTasks|PendingTasks|afterRenderEffect|linkedSignal" --type ts packages/angular-query-experimental/src/ 2>/dev/null | head -20
echo -e "\n=== Checking for Angular v19 release notes specific patterns ==="
rg -n "inject|effect|computed|signal" --type ts packages/angular-query-experimental/src/ 2>/dev/null | head -25Repository: TanStack/query
Length of output: 8462
🌐 Web query:
PendingTasks Angular API introduced version
💡 Result:
PendingTasks (the stable @angular/core API) is stable since Angular v20.0. [1]
Before that, Angular exposed it as ExperimentalPendingTasks starting in Angular v18 (experimental), and later migrated/stabilized it into PendingTasks. [2]
🌐 Web query:
linkedSignal Angular API introduced version
💡 Result:
linkedSignal() was introduced in Angular v19 as a Developer Preview API. [1]
It became stable in Angular v20.0 (“stable since v20.0”). [2]
Sources: [1], [2]
Fix peer dependency: requires >=20.0.0, not >=19.0.0
The code uses PendingTasks and linkedSignal APIs from @angular/core:
PendingTasksis only available (stable) in Angular v20+. It does not exist in v19, so imports will fail.linkedSignalis available in v19 as Developer Preview but is only stable in v20.
The peer dependency range >=19.0.0 is incompatible with the actual implementation. Either bump the peer dependency to >=20.0.0 or add conditional logic to handle Angular v19 (falling back to ExperimentalPendingTasks if available). As written, consumers on Angular 19 will encounter runtime import errors.
🤖 Prompt for AI Agents
In `@packages/angular-query-experimental/package.json` around lines 104 - 107, The
peerDependencies field currently allows `@angular/core` and `@angular/common`
>=19.0.0 but the code imports and uses `@angular/core` APIs only stable in v20
(PendingTasks and linkedSignal); update package.json peerDependencies to require
">=20.0.0" for both `@angular/core` and `@angular/common` so consumers on Angular 19
cannot install an incompatible version, or alternatively implement a runtime
shim in the module that detects Angular v19 and falls back to
ExperimentalPendingTasks/preview linkedSignal imports before using
PendingTasks/linkedSignal; locate uses of PendingTasks and linkedSignal in the
codebase to ensure the fallback or the package.json change covers all imports.
There was a problem hiding this comment.
With 19 ending it's LTS in 3 months, maybe the PR could aim to support angular 20 and up instead 🤔
There was a problem hiding this comment.
🧩 Analysis chain
@benjavicente That makes sense! Angular 19 transitions to LTS on May 19, 2025, with LTS ending on May 19, 2026, so targeting Angular 20+ is a pragmatic choice that avoids the complexity of maintaining compatibility shims for APIs that are only stable in v20.
🏁 Script executed:
#!/bin/bash
# Find all documentation and code references to Angular v19 or version 19
echo "=== Checking for Angular v19 references in the PR changes ==="
rg -n "(?i)(angular\s+v?19|version\s+19|>=\s*19\.0\.0)" --type md --type json packages/angular-query-experimental/ docs/framework/angular/ 2>/dev/null | head -40
echo -e "\n=== Checking README for version references ==="
cat packages/angular-query-experimental/README.md | grep -n "19\|version"
Length of output: 1372
Perfect! I found all the places that need updating. Let me create a plan to update Angular v19 references to v20:
Update Angular version requirement from v19 to v20
Since the codebase uses Angular v20-only APIs (PendingTasks and linkedSignal), and Angular 19 LTS ends in ~3 months, we'll update all version references to require Angular v20+.
- Update package.json peerDependencies
- Update README and documentation files
- Ensure consistency across all references
🏁 Script executed:
#!/bin/bash
# Check if there are any other v19 references in the PR changes that we might have missed
echo "=== Double-checking for any remaining v19 references ==="
rg -n "(?i)(v19|version 19|angular 19|\b19\.0|\b19\+)" --type md packages/angular-query-experimental/ docs/framework/angular/ 2>/dev/null | grep -v "node_modules" | head -30
Length of output: 610
Great! The double-check confirms we've found all the references. The plan above covers all 6 locations that need updating:
- package.json - peerDependencies (the critical fix)
- README.md - documentation
- installation.md - documentation
- overview.md - documentation
- zoneless.md - documentation (specifically mentions PendingTasks)
These changes will:
- ✅ Fix the peer dependency mismatch flagged by CodeRabbit
- ✅ Ensure users on Angular 19 can't install an incompatible version
- ✅ Keep documentation consistent with actual requirements
- ✅ Future-proof the package as Angular 19 LTS support ends in ~3 months
The implementation uses Angular v20-stable APIs (PendingTasks and linkedSignal), so requiring v20+ is the right approach. 🎯
🧠 Learnings used
Learnt from: TkDodo
Repo: TanStack/query PR: 9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.184Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
💡 For code changes across multiple files, use @coderabbitai plan in an issue comment
packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts
Outdated
Show resolved
Hide resolved
packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts
Outdated
Show resolved
Hide resolved
packages/angular-query-experimental/src/__tests__/inject-queries.test.ts
Outdated
Show resolved
Hide resolved
packages/angular-query-experimental/src/__tests__/zonejs-adapter.test.ts
Outdated
Show resolved
Hide resolved
…nput signal too soon
…nal based adapters
benjavicente
force-pushed
the
angular-19
branch
from
50fa064 to
0c2f8ad
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/framework/angular/guides/invalidations-from-mutations.md (1)
25-35:⚠️ Potential issue | 🟠 MajorAdd missing Angular imports to keep the example compile‑ready.
@Componentandinjectare used but not imported, so the snippet won't compile as-is.✅ Suggested fix
import { injectMutation, QueryClient, } from '@tanstack/angular-query-experimental' +import { Component, inject } from '@angular/core'packages/angular-query-experimental/src/types.ts (1)
111-130:⚠️ Potential issue | 🟠 Major
CreateInfiniteQueryResultandDefinedCreateInfiniteQueryResultare missing the same key-omitting pattern used in their base query counterparts.
CreateBaseQueryResultandDefinedCreateQueryResultboth omitBaseQueryNarrowingkeys before mapping signals (OmitKeyof<TState, keyof BaseQueryNarrowing, 'safely'>), but:
CreateInfiniteQueryResult(line 104) intersectsBaseQueryNarrowingyet passesInfiniteQueryObserverResultdirectly toMapToSignalswithout theOmitKeyofstepDefinedCreateInfiniteQueryResult(line 111) doesn't intersect withBaseQueryNarrowingat all, unlikeDefinedCreateQueryResultThis inconsistency should be resolved to maintain uniform type narrowing semantics across all query result types.
🧹 Nitpick comments (13)
docs/framework/angular/guides/testing.md (1)
10-10: Clarify the zoneless TestBed setup claim.The text states "The examples below use a zoneless TestBed setup" but the examples don't show explicit zoneless configuration (e.g.,
provideExperimentalZonelessChangeDetection()). Consider either:
- Adding the explicit zoneless provider to the first example (lines 16-28), or
- Removing the "zoneless" claim if the examples work in both zoned and zoneless contexts
This will help readers understand whether zoneless mode is required or optional for the patterns shown.
packages/angular-query-experimental/src/inject-queries.ts (2)
431-435: Proxy array is fully rebuilt on every query result update.
proxiedResultsSignalis acomputedoverresultSignal(), so every time any query result changes, allsignalProxywrappers (and their internalcomputedsignals) are discarded and recreated. For a small number of queries this is fine, but it scales as O(N) allocations per update.Consider caching proxies by index and only growing/shrinking the array when the query count changes, similar to how
createResultProxyalready accepts a stable index:♻️ Sketch: reuse proxies across updates
- const proxiedResultsSignal = computed(() => - (resultSignal() as Array<QueryObserverResult>).map((_, index) => - createResultProxy(index), - ), - ) + const proxyCache: Array<ReturnType<typeof createResultProxy>> = [] + + const proxiedResultsSignal = computed(() => { + const results = resultSignal() as Array<QueryObserverResult> + // Grow cache as needed; existing proxies already read by index from resultSignal + while (proxyCache.length < results.length) { + proxyCache.push(createResultProxy(proxyCache.length)) + } + // Shrink if queries were removed + proxyCache.length = results.length + return proxyCache.slice() + })Since
createResultProxy(index)already readsresultSignal()[index]reactively, cached proxies remain correct across updates.
442-452: Type erasure viaas unknown as TCombinedResulton both branches.When
combineis present,resultSignal()is returned directly (alreadyTCombinedResult). When absent,proxiedResultsSignal()is returned — this isArray<MapToSignals<...>>which matchesQueriesResults<T>(the default forTCombinedResult). The double cast at line 452 covers both paths. This is acceptable given the runtime-level proxy wrapping that satisfies the contract, but worth a brief inline comment noting why the cast is safe for the non-combine branch.packages/angular-query-experimental/src/__tests__/test-utils.ts (1)
74-99:registerSignalInputrelies on private Angular APIs (ɵcmp/ɵdir) — the temporary-use comment is appreciated.The comment at Line 80-81 correctly notes this should become obsolete with Angular 21's CLI support for Vitest. This is a pragmatic workaround for testing signal-based inputs without the Angular compiler.
One nit: the
SIGNAL_BASED_INPUT_FLAG = 1constant relies on Angular's internal input encoding. If the internal representation changes in a future Angular version, this will silently break.packages/angular-query-experimental/src/__tests__/inject-queries.test.ts (1)
368-442: Well-structured test for dynamic query array changes with optimistic transitions.The test correctly verifies:
- Initial data loading across 3 queries
- Shrinking the queries array from 3 → 2, including checking for optimistic transition snapshots
- Emptying the array entirely
The module-level
queriessignal (Line 398) vs the instancequeriesproperty (Line 376) naming overlap is a bit confusing but works correctly since the class body references the module-level signal withoutthis..Consider renaming the module-level signal (e.g.,
queryInputs) to avoid shadowing the component property name, improving readability.packages/angular-query-experimental/src/inject-mutation.ts (1)
140-181: Minor inconsistency:destroyedflag managed in effect cleanup vs.DestroyRef.onDestroyincreate-base-query.ts.In
inject-mutation.ts, thedestroyedflag is set in the effect'sonCleanupcallback (Line 172). Increate-base-query.ts(Line 191-194), a separatedestroyRef.onDestroy()handler is used. Both approaches work because the effect is tied to the injector's lifecycle, but the inconsistency could be confusing for maintainers.Consider aligning both files to use the same pattern for consistency.
packages/angular-query-experimental/src/create-base-query.ts (2)
129-167: Verify timing: pending task tracking is synchronous but result updates are deferred viaqueueMicrotask.In
subscribeToObserver,startPendingTask()/stopPendingTask()execute synchronously in the observer callback (Lines 137-141), butresultSignal.set()is deferred to a microtask (Line 143). This split means Angular'sPendingTasksreflects pending state immediately, while the result signal lags by one microtask.This is likely intentional — Angular needs to know work is pending before the result updates — but it introduces a narrow window where
fetchStatuscould flip (e.g., idle → fetching → idle) and the pending task gets stopped before the intermediate result is published. For most real-world usage this is fine, but it's worth being aware of during SSR/whenStable()testing.Consider adding a brief comment near Lines 137-141 documenting that the synchronous pending task tracking is intentional for
whenStable()integration, and that result updates are deliberately deferred.
48-48:excludeFunctionstyped asReadonlyArray<string>loses compile-time safety.The parameter is declared as
ReadonlyArray<string>but callers pass typedArray<MethodKeys<...>>. Consider typing the parameter more strictly to catch mismatches at compile time:Proposed change
- excludeFunctions: ReadonlyArray<string>, + excludeFunctions: ReadonlyArray<MethodKeys<QueryObserverResult<TData, TError>>>,This would eliminate the cast at Line 198 and prevent accidentally passing non-method keys.
packages/angular-query-experimental/src/__tests__/pending-tasks.test.ts (3)
382-385:fixture.detectChanges()afterfixture.destroy()may be a no-op.After destroying a component fixture, calling
detectChanges()on it is typically inert (the view is already detached). The assertion on line 385 likely passes becausedestroy()itself cleans up pending tasks, not because of the subsequentdetectChanges(). Consider removing the redundant call for clarity.Suggested change
// Destroy component while mutation is running fixture.destroy() - fixture.detectChanges() expect(fixture.isStable()).toBe(true)
500-534: MissingTestBed.tick()aftermutation.mutate()— inconsistent with other mutation tests.Other mutation tests in this file (lines 117, 142, 485) call
TestBed.tick()aftermutate()to trigger Angular's change detection and pending task registration. This test omits it. It likely works becauseadvanceTimersByTimeAsync(70)covers the timing, but the inconsistency could lead to flaky behavior if the internal scheduling changes.
334-354: Component defined atdescribescope uses inline class withoutselector.The
TestComponentat line 335 omits theselectorproperty in@Component, while other inline test components in this file includeselector: 'app-test'. This works since Angular doesn't require a selector for dynamically-created components, but it's a minor inconsistency with the rest of the file.packages/angular-query-experimental/src/__tests__/inject-query.test.ts (1)
582-624: Robust error capture pattern, butprocess.on('uncaughtException')may be fragile.The test captures errors via both
zone.onErrorandprocess.on('uncaughtException'). Thefinallyblock correctly cleans up the process listener. However, if another test in the same process throws an unhandled exception concurrently (e.g., due to parallel test execution), this listener could capture the wrong error. This is likely acceptable in practice since Vitest runs tests in the same file serially.packages/angular-query-experimental/src/signal-proxy.ts (1)
29-46: Falsy guard on cached fields could return prototype properties prematurely.Line 32:
if (computedField) return computedFielduses a truthy check. Sincetargetis{} as MapToSignals<...>, accessing inherited prototype properties likeconstructorortoStringwill be truthy and short-circuit the proxy logic. This is unlikely to matter in practice (consumers access query result fields, not prototype methods), but a stricter check would be more defensive.Suggested defensive fix
get(target, prop) { - // first check if we have it in our internal state and return it - const computedField = target[prop] - if (computedField) return computedField + // first check if we have it in our internal state and return it + if (Object.prototype.hasOwnProperty.call(target, prop)) { + return target[prop] + }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/angular-query-experimental/src/__tests__/inject-queries.test.ts`:
- Around line 148-153: In the combine function, avoid returning a value from the
forEach callback — replace the arrow shorthand (r) => r.refetch() inside
queryResults.forEach(...) with a block body that calls r.refetch() without
returning (e.g. (r) => { r.refetch(); }) or, if the intent is to await all
refetches, replace forEach with Promise.all(queryResults.map((r) =>
r.refetch())) so refetch promises are handled; update the combine implementation
accordingly (refer to combine, queryResults.forEach, and refetch).
🧹 Nitpick comments (1)
packages/angular-query-experimental/src/__tests__/inject-query.test.ts (1)
582-624: Robust but fragile error-catching setup — consider documenting the pattern.The dual
zone.onError+process.on('uncaughtException')approach works but is intricate. If a future contributor changes the error propagation path, both channels need to fire. A brief inline comment explaining why both are needed (zone re-throws into the Node process) would help maintainability.
| combine: (queryResults) => { | ||
| return { | ||
| refetch: () => queryResults.forEach((r) => r.refetch()), | ||
| data: queryResults.map((r) => r.data).join(','), | ||
| } | ||
| }, |
There was a problem hiding this comment.
Avoid returning a value from forEach callback.
The arrow shorthand (r) => r.refetch() implicitly returns the Promise from refetch(), which forEach discards. Biome flags this as suspicious.
Proposed fix
combine: (queryResults) => {
return {
- refetch: () => queryResults.forEach((r) => r.refetch()),
+ refetch: () => queryResults.forEach((r) => { r.refetch() }),
data: queryResults.map((r) => r.data).join(','),
}
},📝 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.
| combine: (queryResults) => { | |
| return { | |
| refetch: () => queryResults.forEach((r) => r.refetch()), | |
| data: queryResults.map((r) => r.data).join(','), | |
| } | |
| }, | |
| combine: (queryResults) => { | |
| return { | |
| refetch: () => queryResults.forEach((r) => { r.refetch() }), | |
| data: queryResults.map((r) => r.data).join(','), | |
| } | |
| }, |
🧰 Tools
🪛 Biome (2.3.13)
[error] 150-150: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🤖 Prompt for AI Agents
In `@packages/angular-query-experimental/src/__tests__/inject-queries.test.ts`
around lines 148 - 153, In the combine function, avoid returning a value from
the forEach callback — replace the arrow shorthand (r) => r.refetch() inside
queryResults.forEach(...) with a block body that calls r.refetch() without
returning (e.g. (r) => { r.refetch(); }) or, if the intent is to await all
refetches, replace forEach with Promise.all(queryResults.map((r) =>
r.refetch())) so refetch promises are handled; update the combine implementation
accordingly (refer to combine, queryResults.forEach, and refetch).
2 participants
🎯 Changes
This is a continuation of #9893, where the main new changes added are:
data = query.datato fail since it would read the input signal to do the check, and in Angular input signals can't be read in the component initialization.DefinedInitialDataOptions, removing the override ofqueryFn. This change fixes the select function on queries and aligns the implementation with the rest of the signal based adapters.I also added more tests, changed some files and type tests that aren't needed in angular v19 and up, and went thought the docs to improve the consistencies and recommendations given the current implementation.
For the docs, the main improvements were removing the recommendation of discriminating with the status signal since it is not type safe in this adapter, removing examples and a note that hinted on RxJS support for query functions, and adding missing
this.and signal calls on some examples.~Before reviews, I want to add new tests, specially for the
createBaseQuerychanges on #9893, where the value is ready after effects instead on after init. So I'm leaving this PR as draft until that. ~Edit (08/02): I ussed AI/Codex to give it another cleanup, and reviewed all of the generated code. I don't expect this PR to be merged as is, specially considering the amount of changes. Moved the injectQueries out of experimental after a refractor to follow the same system of
injectQuery+ new tests. Also, I gave codex access to read issues on this repo, and in a sample project, it managed to verify that most if not all of the reported issues are resolved with the changes of this PR.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Breaking Changes
Documentation
Tests