refactor(angular-query): require Angular v19+ and rely on component effect schedule timing #9893
Conversation
🦋 Changeset detectedLatest commit: 75cb8ca 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 |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis changeset modernizes the Angular Query experimental package for Angular 19+, replacing the PendingTasks compatibility shim with Angular's native API, refactoring test suites to use standalone components with OnPush change detection, restructuring provider functions to return EnvironmentProviders, and updating core query/mutation lifecycle management to use Angular's component effect scheduling. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 75cb8ca
☁️ Nx Cloud last updated this comment at |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9893 +/- ##
===========================================
+ Coverage 45.78% 94.48% +48.69%
===========================================
Files 200 20 -180
Lines 8413 435 -7978
Branches 1922 99 -1823
===========================================
- Hits 3852 411 -3441
+ Misses 4113 23 -4090
+ Partials 448 1 -447
🚀 New features to boost your workflow:
|
arnoud-dv
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
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/__tests__/inject-query.test.ts (1)
712-983: Add direct-call test variant to properly exercise "outside injection context" behaviorThe current test at line 712 instantiates a component inside TestBed, which provides an injection context. This means
injectQueryis called within an existing injection context, not outside it—unlike the pattern used consistently acrossinjectMutation,injectIsFetching,injectIsMutating, andinjectMutationStatetests.To match the established pattern and properly verify that passing
{ injector }allows usage outside an injection context, add a direct-call variant alongside the component-based test:const injector = TestBed.inject(Injector) expect(() => injectQuery( () => ({ queryKey: ['manualInjector'], queryFn: () => sleep(0).then(() => 'Some data'), }), { injector }, ), ).not.toThrow()The new PendingTasks-oriented tests (
whenStable(), synchronousqueryFn,staleTime, enabled transitions, and invalidation) are correctly implemented per Angular 20 testing guidance.
🧹 Nitpick comments (6)
examples/angular/optimistic-updates/src/app/components/optimistic-updates.component.ts (1)
39-41: Tracking by$indexis acceptable for this demoUsing
track $indexis fine here since the list is effectively append-only and tasks are simple values. Be aware this will remount items on reordering; if you ever introduce reordering or stable ids, consider tracking by a unique task identifier instead.packages/angular-query-experimental/src/providers.ts (1)
1-9: Consider flattening feature providers before callingmakeEnvironmentProviders
provideTanStackQuerynow returnsEnvironmentProviders, which is good. However, the current call:return makeEnvironmentProviders([ provideQueryClient(queryClient), features.map((feature) => feature.ɵproviders), ])builds a nested structure (
Provider | Array<Provider | EnvironmentProviders>). While Angular DI usually flattens provider arrays at runtime, this shape is a bit awkward for TypeScript and diverges from common patterns.To align more closely with Angular’s own helpers and keep types clean, consider flattening:
export function provideTanStackQuery( queryClient: QueryClient | InjectionToken<QueryClient>, ...features: Array<QueryFeatures>, ): EnvironmentProviders { const featureProviders = features.flatMap((feature) => feature.ɵproviders) return makeEnvironmentProviders([ provideQueryClient(queryClient), ...featureProviders, ]) }This also fits well with the widened
ɵproviders: Array<Provider | EnvironmentProviders>and keepsQueryFeaturecomposition straightforward.Please verify this against the current
makeEnvironmentProviderssignature and Angular’s own helper implementations (e.g.provideRouter,provideHttpClient) to ensure consistent usage.Also applies to: 110-118, 129-132, 142-145, 153-156
packages/angular-query-experimental/src/devtools/with-devtools.ts (1)
11-12: Devtools env-initializer and lifecycle management look solid; refine missing-client error pathThe move to
provideEnvironmentInitializerwith a signal-driveneffectis well-structured:
- Browser-only guard and
DEVTOOLS_PROVIDEDflag prevent duplicate initialization.shouldLoadToolsSignal+ effect correctly handle “load, update, destroy” behavior as options change.- Dynamic import is safely guarded by
injectorIsDestroyed, and devtools teardown is wired intoDestroyRef.One small nit:
getResolvedQueryClient()throws'No QueryClient found', but that error is funneled into the same.catchblock that logs:'Install @tanstack/query-devtools or reinstall without --omit=optional.'This message is misleading when the real issue is a missing
QueryClientrather than a missing devtools package.Consider branching in the
catchto distinguish these cases, e.g.:.catch((error: unknown) => { if (error instanceof Error && error.message === 'No QueryClient found') { console.error( 'Angular Query Devtools: No QueryClient was found. ' + 'Provide one via provideTanStackQuery or via devtools options.', error, ) return } console.error( 'Install @tanstack/query-devtools or reinstall without --omit=optional.', error, ) })This will make configuration issues much easier to diagnose.
Please confirm this behavior against your intended public docs for
withDevtoolsso the error messaging matches how you expect users to wire up theirQueryClient.Also applies to: 13-14, 61-71, 72-112, 113-169
packages/angular-query-experimental/src/create-base-query.ts (1)
2-10: Observer lifecycle, PendingTasks, and microtask batching are wired correctly; tiny naming nitThis refactor in
createBaseQuerylooks well thought out:
PendingTasksintegration viastartPendingTask/stopPendingTaskties task lifetime tofetchStatusand ensures cleanup on both subscription teardown andDestroyRef.onDestroy.queueMicrotaskaroundnotifyManager.batchplusngZone.runshould align query notifications with Angular’s component effect scheduling and avoid the optimistic-update flicker you were seeing.linkedSignalkeyed bydefaultedOptionsSignal, together withtrackObserverResult/autoTrackResultProperties, maintainsnotifyOnChangePropssemantics while still allowing optimistic results.OBSERVER_NOT_READY_ERRORis a good defensive guard against reading query state during construction; it will surface misuses clearly.Only minor nit: the error message
'injectQuery: QueryObserver not initialized yet. Avoid reading the query result during construction'is slightly misleading when
createBaseQuerybacks bothinjectQueryandinjectInfiniteQuery. If you ever touch this again, consider a more generic wording (e.g. “Angular Query injection: …”) to avoid confusion in stack traces.Overall, the lifecycle and batching changes look consistent with the PR objectives.
Given the subtle timing behavior this is meant to fix, please re-run the original optimistic-update repro (issue #9735) and any SSR/restoration scenarios you have to confirm the microtask-based batching behaves as intended across Angular 19–21.
Also applies to: 48-75, 91-147, 148-185, 186-215, 216-217
packages/angular-query-experimental/src/__tests__/inject-query.test.ts (2)
345-520: Reactive options tests look good; consider explicitdetectChanges()after enablingThe signal-driven tests (
options contained signal change,enabledsignal, dependent queries, and dynamicqueryKeyforrefetch) are nicely migrated to component-scoped queries and make good use of external signals + class fields to verify that options are recomputed under Angular signals.One small robustness tweak: in
test('should only run query once enabled signal is set to true'), the query’senabledoption depends onthis.enabled()but afterenabled.set(true)there is no explicitfixture.detectChanges(). While this likely works today becauseinjectQuerytracks signals directly, adding a change detection call would make the test less coupled to internal implementation details:@@ - enabled.set(true) - - await vi.advanceTimersByTimeAsync(11) + enabled.set(true) + fixture.detectChanges() + + await vi.advanceTimersByTimeAsync(11)This keeps behavior the same while making the dependency on Angular’s re-evaluation semantics explicit. Based on learnings.
522-645:throwOnErrortests are thorough; watch globalprocesshandler usageThe updated
throwOnErrortests now:
- Verify callback-based
throwOnErrorusing an OnPush component.- For the “throws” variants, hook both
NgZone.onErrorandprocess.on('uncaughtException')and assert they receive the expected error, cleaning up theprocesslistener in both the handler andfinallyblock.This gives strong coverage of how errors escape into Angular and Node. The only caveat is that relying on
process.on('uncaughtException')is globally scoped; if other tests ever emit uncaught exceptions while these promises are pending, it could cause cross-test interference. If that becomes an issue, you might want to gate these tests behind a dedicated runner or add some defensive checks in the handler to ensure the error matches the expected shape/message before resolving.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.changeset/deep-crews-open.md(1 hunks)examples/angular/infinite-query-with-max-pages/src/app/components/example.component.ts(1 hunks)examples/angular/optimistic-updates/src/app/components/optimistic-updates.component.ts(1 hunks)packages/angular-query-experimental/README.md(1 hunks)packages/angular-query-experimental/package.json(1 hunks)packages/angular-query-experimental/src/__tests__/inject-infinite-query.test.ts(3 hunks)packages/angular-query-experimental/src/__tests__/inject-mutation-state.test.ts(2 hunks)packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts(3 hunks)packages/angular-query-experimental/src/__tests__/inject-queries.test.ts(2 hunks)packages/angular-query-experimental/src/__tests__/inject-query.test.ts(20 hunks)packages/angular-query-experimental/src/__tests__/pending-tasks.test.ts(8 hunks)packages/angular-query-experimental/src/create-base-query.ts(4 hunks)packages/angular-query-experimental/src/devtools/with-devtools.ts(2 hunks)packages/angular-query-experimental/src/inject-mutation.ts(4 hunks)packages/angular-query-experimental/src/pending-tasks-compat.ts(0 hunks)packages/angular-query-experimental/src/providers.ts(5 hunks)
💤 Files with no reviewable changes (1)
- packages/angular-query-experimental/src/pending-tasks-compat.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
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-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.jsonpackages/angular-query-experimental/src/inject-mutation.tspackages/angular-query-experimental/src/create-base-query.tspackages/angular-query-experimental/src/providers.ts
📚 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:
.changeset/deep-crews-open.mdpackages/angular-query-experimental/src/inject-mutation.tspackages/angular-query-experimental/src/__tests__/inject-infinite-query.test.tspackages/angular-query-experimental/src/__tests__/inject-query.test.tspackages/angular-query-experimental/src/__tests__/pending-tasks.test.tspackages/angular-query-experimental/src/create-base-query.ts
📚 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:
.changeset/deep-crews-open.mdpackages/angular-query-experimental/src/__tests__/inject-queries.test.tspackages/angular-query-experimental/src/__tests__/inject-infinite-query.test.tspackages/angular-query-experimental/src/__tests__/inject-query.test.tspackages/angular-query-experimental/src/__tests__/pending-tasks.test.ts
📚 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:
packages/angular-query-experimental/src/devtools/with-devtools.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (15)
packages/angular-query-experimental/README.md (1)
32-32: Clear and well-placed version requirement notice.The Quick Start update effectively communicates the Angular 19+ requirement before installation steps, preventing compatibility issues upfront.
.changeset/deep-crews-open.md (1)
1-5: Well-structured changeset entry.The format and content are correct: minor version bump with a clear summary linking the version requirement to the underlying architectural change (component effect scheduling).
packages/angular-query-experimental/package.json (1)
108-109: Peer dependency constraints are correct and well-scoped.The constraints (
>=19.0.0) properly enforce the Angular 19+ requirement while allowing compatible future versions. DevDependencies at^20.0.0provide a reasonable development environment aligned with the peer constraint.packages/angular-query-experimental/src/inject-mutation.ts (1)
4-11: PendingTasks integration and cleanup look correctThe switch to Angular’s
PendingTasksplustaskCleanupRefworks as expected: you add once whenstate.isPendingbecomes true, clear on resolution, and also clean up on effect teardown. This keeps the pending-task count accurate and avoids leaks even if the effect re-runs or unsubscribes while pending.Please double-check against the Angular 19+/20
PendingTasks.add()contract (return type and availability) for all supported targets, just to be sure there are no surprising platform differences.Also applies to: 61-63, 128-144, 161-164
packages/angular-query-experimental/src/__tests__/inject-mutation-state.test.ts (1)
2-8: OnPush change detection onFakeComponentlooks correctUsing
ChangeDetectionStrategy.OnPushonFakeComponentwhile driving it viasetFixtureSignalInputsand explicitfixture.detectChanges()keeps the test closer to real-world usage without changing its assertions. No issues spotted.Also applies to: 142-181
packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts (1)
3-9: OnPush for mutation input tests is consistent and safeThe added
ChangeDetectionStrategy.OnPushon the fake components used in the required-signal-input tests fits the new testing pattern (standalone + OnPush + explicitdetectChanges()) and doesn't change the mutation semantics. Looks good.Also applies to: 305-341
packages/angular-query-experimental/src/__tests__/inject-queries.test.ts (1)
4-8: OnPush onPagecomponent keeps the test aligned with production usageSwitching the test
Pagecomponent toChangeDetectionStrategy.OnPushwhile still rendering it via@testing-library/angularmaintains the intended behavior of theinjectQuerieseffect-driven snapshot while aligning with the library's recommended pattern.Also applies to: 32-42
packages/angular-query-experimental/src/__tests__/inject-infinite-query.test.ts (1)
31-78: Component-scoped infinite query test matches the new Angular integrationRefactoring the infinite-query test to a standalone, OnPush
TestComponentwhosequeryfield usesinjectInfiniteQueryand is accessed via a fixture keeps the assertions identical while better exercising the real integration (component effects, PendingTasks, fake timers). This pattern looks solid.packages/angular-query-experimental/src/__tests__/pending-tasks.test.ts (5)
55-89: Synchronous queryFn +whenStable()test correctly exercises PendingTasksWrapping the synchronous
queryFnin a standalone, OnPush component and assertingstatus: 'pending'beforeapp.whenStable()then'success'afterward ensures the PendingTasks integration still treats synchronous resolutions as async from Angular’s perspective. The flow and timer usage look sound.
195-231: Rapid refetch test nicely validates PendingTasks with side-effectful queryFnThe new
TestComponentwithcallCountand anasyncqueryFn(incrementingcallCountthen awaitingsleep) is a good fit for validating that multiple rapidrefetch()calls do not leak PendingTasks. Usingawait vi.advanceTimersByTimeAsync(20)plusapp.whenStable()should flush all attempts deterministically. This aligns with the async/await style for side-effectful queryFns. Based on learnings.
233-279: Offline-start test robustly covers paused queries and PendingTasks blockingThe new
"start-offline"test that:
- sets
onlineManageroffline,- initializes an
injectQuerywithnetworkMode: 'online',- verifies
status: 'pending'+fetchStatus: 'paused', and- checks that
app.whenStable()does not resolve until the app comes back onlineis a good regression test for PendingTasks behavior when queries never enter the
fetchingstate. The microtask and timer sequencing looks appropriate.
348-403: Component destruction tests correctly combine OnPush and PendingTasks cleanupAdding
ChangeDetectionStrategy.OnPushtoTestComponentand callingfixture.detectChanges()before starting the query/mutation ensures the operations are actually in-flight when the component is destroyed. The subsequentapp.whenStable()expectations validate that PendingTasks entries are cleaned up even if queries/mutations are torn down mid-flight. This is a solid coverage improvement.
406-459: Concurrent queries component test aligns well with new effect schedulingThe new standalone, OnPush
TestComponenthosting three concurrent queries (two async, one synchronous) and driving them through a fixture gives good coverage of concurrent PendingTasks behavior in a realistic component context. The expectations before and afterapp.whenStable()look correct and should catch regressions in scheduling or task cleanup.packages/angular-query-experimental/src/__tests__/inject-query.test.ts (2)
52-260: Type inference test via standalone OnPush component looks correctMoving the “should return the correct types” assertions into a standalone, OnPush
TestComponentthat hosts a variety ofinjectQueryusages keeps theexpectTypeOfchecks intact while aligning with the rest of the test suite’s component-based setup. Although a few queryFns (e.g. thefetch('return Promise<any')case) will run at runtime, they don’t influence the compile-time type assertions and mirror the previous behavior.
262-343: Basic pending/success/error signal tests are well-adapted to the component patternThe refactored tests for:
- initial pending state,
- successful resolution, and
- rejection with appropriate error state
now use standalone, OnPush components with
queryas a field plus fixture-drivendetectChanges(). The timer usage and assertions onstatus,data, and the variousis*helpers correctly exercise the adapter’s behavior under the new effect scheduling.
| readonly nextButtonDisabled = computed( | ||
| () => !this.#hasNextPage() || this.#isFetchingNextPage(), | ||
| () => !this.query.hasNextPage() || this.query.isFetchingNextPage(), | ||
| ) | ||
|
|
||
| readonly nextButtonText = computed(() => | ||
| this.#isFetchingNextPage() | ||
| this.query.isFetchingNextPage() | ||
| ? 'Loading more...' | ||
| : this.#hasNextPage() | ||
| : this.query.hasNextPage() | ||
| ? 'Load newer' | ||
| : 'Nothing more to load', | ||
| ) | ||
|
|
||
| readonly previousButtonDisabled = computed( | ||
| () => !this.#hasPreviousPage() || this.#isFetchingNextPage(), | ||
| () => !this.query.hasPreviousPage() || this.query.isFetchingNextPage(), | ||
| ) | ||
| readonly previousButtonText = computed(() => | ||
| this.#isFetchingPreviousPage() | ||
| this.query.isFetchingPreviousPage() | ||
| ? 'Loading more...' | ||
| : this.#hasPreviousPage() | ||
| : this.query.hasPreviousPage() | ||
| ? 'Load Older' | ||
| : 'Nothing more to load', | ||
| ) |
There was a problem hiding this comment.
Fix previousButtonDisabled to use isFetchingPreviousPage()
previousButtonDisabled currently checks this.query.isFetchingNextPage(), while the text and behavior for the previous button use isFetchingPreviousPage(). This likely means the "Previous" button can remain enabled while a previous-page fetch is in-flight, or be disabled for an unrelated next-page fetch.
Consider aligning the disabled logic with the previous-page fetch state:
- readonly previousButtonDisabled = computed(
- () => !this.query.hasPreviousPage() || this.query.isFetchingNextPage(),
- )
+ readonly previousButtonDisabled = computed(
+ () => !this.query.hasPreviousPage() || this.query.isFetchingPreviousPage(),
+ )📝 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.
| readonly nextButtonDisabled = computed( | |
| () => !this.#hasNextPage() || this.#isFetchingNextPage(), | |
| () => !this.query.hasNextPage() || this.query.isFetchingNextPage(), | |
| ) | |
| readonly nextButtonText = computed(() => | |
| this.#isFetchingNextPage() | |
| this.query.isFetchingNextPage() | |
| ? 'Loading more...' | |
| : this.#hasNextPage() | |
| : this.query.hasNextPage() | |
| ? 'Load newer' | |
| : 'Nothing more to load', | |
| ) | |
| readonly previousButtonDisabled = computed( | |
| () => !this.#hasPreviousPage() || this.#isFetchingNextPage(), | |
| () => !this.query.hasPreviousPage() || this.query.isFetchingNextPage(), | |
| ) | |
| readonly previousButtonText = computed(() => | |
| this.#isFetchingPreviousPage() | |
| this.query.isFetchingPreviousPage() | |
| ? 'Loading more...' | |
| : this.#hasPreviousPage() | |
| : this.query.hasPreviousPage() | |
| ? 'Load Older' | |
| : 'Nothing more to load', | |
| ) | |
| readonly nextButtonDisabled = computed( | |
| () => !this.query.hasNextPage() || this.query.isFetchingNextPage(), | |
| ) | |
| readonly nextButtonText = computed(() => | |
| this.query.isFetchingNextPage() | |
| ? 'Loading more...' | |
| : this.query.hasNextPage() | |
| ? 'Load newer' | |
| : 'Nothing more to load', | |
| ) | |
| readonly previousButtonDisabled = computed( | |
| () => !this.query.hasPreviousPage() || this.query.isFetchingPreviousPage(), | |
| ) | |
| readonly previousButtonText = computed(() => | |
| this.query.isFetchingPreviousPage() | |
| ? 'Loading more...' | |
| : this.query.hasPreviousPage() | |
| ? 'Load Older' | |
| : 'Nothing more to load', | |
| ) |
🤖 Prompt for AI Agents
In
examples/angular/infinite-query-with-max-pages/src/app/components/example.component.ts
around lines 32 to 53, the previousButtonDisabled computed currently checks
this.query.isFetchingNextPage() instead of using
this.query.isFetchingPreviousPage(), causing the Previous button's disabled
state to be tied to next-page fetches; update the computed to call
isFetchingPreviousPage() so the disabled state aligns with previous-page fetch
activity and remains consistent with previousButtonText and the intended
behavior.
| test('can be used outside injection context when passing an injector', () => { | ||
| const query = injectInfiniteQuery( | ||
| () => ({ | ||
| queryKey: ['manualInjector'], | ||
| queryFn: ({ pageParam }) => | ||
| sleep(0).then(() => 'data on page ' + pageParam), | ||
| initialPageParam: 0, | ||
| getNextPageParam: () => 12, | ||
| }), | ||
| { | ||
| injector: TestBed.inject(Injector), | ||
| }, | ||
| ) | ||
| const injector = TestBed.inject(Injector) | ||
|
|
||
| @Component({ | ||
| selector: 'app-test', | ||
| template: '', | ||
| standalone: true, | ||
| changeDetection: ChangeDetectionStrategy.OnPush, | ||
| }) | ||
| class TestComponent { | ||
| query = injectInfiniteQuery( | ||
| () => ({ | ||
| queryKey: ['manualInjector'], | ||
| queryFn: ({ pageParam }) => | ||
| sleep(0).then(() => 'data on page ' + pageParam), | ||
| initialPageParam: 0, | ||
| getNextPageParam: () => 12, | ||
| }), | ||
| { | ||
| injector: injector, | ||
| }, | ||
| ) | ||
| } | ||
|
|
||
| const fixture = TestBed.createComponent(TestComponent) | ||
| fixture.detectChanges() | ||
| const query = fixture.componentInstance.query | ||
|
|
||
| expect(query.status()).toBe('pending') | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
Injection-context test no longer calls injectInfiniteQuery outside an injection context
In test('can be used outside injection context when passing an injector'), injectInfiniteQuery is now invoked inside a component constructor/field initializer, which already has an Angular injection context. The description suggests we still want to verify the "no injection context but manual injector" path.
To keep that behavior covered, consider adding (or restoring) a direct call pattern alongside the component-based test, for example:
- @Component({
- selector: 'app-test',
- template: '',
- standalone: true,
- changeDetection: ChangeDetectionStrategy.OnPush,
- })
- class TestComponent {
- query = injectInfiniteQuery(
- () => ({
- queryKey: ['manualInjector'],
- queryFn: ({ pageParam }) =>
- sleep(0).then(() => 'data on page ' + pageParam),
- initialPageParam: 0,
- getNextPageParam: () => 12,
- }),
- {
- injector: injector,
- },
- )
- }
+ // Ensure calling injectInfiniteQuery outside an Angular injection
+ // context works when an explicit injector is provided.
+ expect(() =>
+ injectInfiniteQuery(
+ () => ({
+ queryKey: ['manualInjector'],
+ queryFn: ({ pageParam }) =>
+ sleep(0).then(() => 'data on page ' + pageParam),
+ initialPageParam: 0,
+ getNextPageParam: () => 12,
+ }),
+ { injector },
+ ),
+ ).not.toThrow()I'll verify this review comment by examining the test file and understanding the injection context behavior.
</script> ```🏁 Script executed:
#!/bin/bash
# Examine the test file to see the full context and surrounding tests
cat packages/angular-query-experimental/src/__tests__/inject-infinite-query.test.tsLength of output: 3294
Add a direct test call outside injection context to match test description
The test "can be used outside injection context when passing an injector" calls injectInfiniteQuery() inside a component's field initializer, which runs within Angular's automatic injection context. This contradicts the test's stated purpose.
The first injection context test correctly demonstrates calling outside a context (and verifies it throws without an injector). The second test should actually verify that providing an explicit injector allows the call to succeed outside a context.
Add a direct call pattern to verify the manual injector works without relying on Angular's context:
test('can be used outside injection context when passing an injector', () => {
const injector = TestBed.inject(Injector)
+ // Ensure calling injectInfiniteQuery outside an Angular injection
+ // context works when an explicit injector is provided.
+ expect(() =>
+ injectInfiniteQuery(
+ () => ({
+ queryKey: ['manualInjector'],
+ queryFn: ({ pageParam }) =>
+ sleep(0).then(() => 'data on page ' + pageParam),
+ initialPageParam: 0,
+ getNextPageParam: () => 12,
+ }),
+ { injector },
+ ),
+ ).not.toThrow()
+
@Component({🤖 Prompt for AI Agents
In
packages/angular-query-experimental/src/__tests__/inject-infinite-query.test.ts
around lines 93 to 122, the test claims to verify calling injectInfiniteQuery
outside an injection context when passing an injector but currently calls it
inside a component field initializer (an automatic injection context). Change
the test to perform a direct call to injectInfiniteQuery outside any component:
obtain the injector via TestBed.inject(Injector), then call
injectInfiniteQuery(...) directly with the injector option (not inside a
component) and assert the returned query behaves as expected (e.g., status() ===
'pending'); remove or replace the component-based call so the test truly
exercises manual injector usage outside Angular's automatic context.
d71c4e3 to
39b8ae7
Compare
github-actions
bot
added
the
documentation
arnoud-dv
force-pushed
the
refactor/require-angular-v19
branch
from
39b8ae7 to
f1d23ff
Compare
arnoud-dv
force-pushed
the
refactor/require-angular-v19
branch
from
f1d23ff to
75cb8ca
Compare
1 participant
Improves reactivity and timing
To benefit from the more mature signal ecosystem and PendingTasks integration in recent Angular versions this drops support for Angular v16-v18. These Angular versions are also not under LTS support anymore, so we keep support for all Angular versions currently under active or LTS support: v19 to v21
🎯 Changes
Breaking Changes
Aliasing query properties on a component will result in an error:
This should be resolved once the signalProxy is replaced.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Release Notes
Breaking Changes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.