fix(angular-query): track pre-render query promises#10779
Conversation
📝 WalkthroughWalkthroughThis PR fixes a bug where accessing a query's data signal before initial render causes Angular's ChangesKeep Angular whenStable() pending when query data accessed early
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/angular-query-experimental/src/create-base-query.ts`:
- Around line 70-75: The pendingTasks.add() ref created for thenable results in
create-base-query.ts can be leaked if the thenable never settles; modify the
thenable handling around result.then(...) to ensure the pendingTaskRef is
released exactly once on cancellation/destruction: if an AbortSignal is present
in the query context, register a one-time abort listener that calls
pendingTaskRef() and removes itself; otherwise hook into the query
cancellation/cleanup path (the same place that cancels the request) to call
pendingTaskRef(); ensure both the result.then fulfillment/rejection handlers and
the cancellation listener invoke pendingTaskRef() idempotently (guarded so it
only runs once) and that any registered listener is removed after release.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5cca70e1-f9cd-4f4f-8a96-a4451e588fd6
📒 Files selected for processing (3)
.changeset/angular-query-preaccess-whenstable.mdpackages/angular-query-experimental/src/__tests__/inject-query.test.tspackages/angular-query-experimental/src/create-base-query.ts
| if (result && typeof result.then === 'function') { | ||
| const pendingTaskRef = pendingTasks.add() | ||
| void result.then( | ||
| () => pendingTaskRef(), | ||
| () => pendingTaskRef(), | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current release paths in create-base-query.ts
rg -n -C3 "pendingTasks\.add|result\.then|context\.signal|abort" packages/angular-query-experimental/src/create-base-query.ts
# Check whether cancellation + never-settling promise is already covered in tests
rg -n -C2 "whenStable|abort|signal|cancel|never" \
packages/angular-query-experimental/src/__tests__/inject-query.test.ts \
packages/angular-query-experimental/src/__tests__/pending-tasks.test.tsRepository: TanStack/query
Length of output: 29091
Potential Angular pending-task leak when wrapped queryFn thenable never settles after cancellation
In packages/angular-query-experimental/src/create-base-query.ts (lines 70-75), the pendingTasks.add() ref is released only inside result.then(...). If cancellation occurs and the returned thenable never settles, that pending task can keep whenStable() pending indefinitely. Add a cancellation/destruction release path for that pendingTaskRef (e.g., release on an AbortSignal if present in the queryFn context, otherwise ensure cleanup triggers the release exactly once).
Segment B (updateState pending-task wiring/cleanup) looks consistent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/angular-query-experimental/src/create-base-query.ts` around lines 70
- 75, The pendingTasks.add() ref created for thenable results in
create-base-query.ts can be leaked if the thenable never settles; modify the
thenable handling around result.then(...) to ensure the pendingTaskRef is
released exactly once on cancellation/destruction: if an AbortSignal is present
in the query context, register a one-time abort listener that calls
pendingTaskRef() and removes itself; otherwise hook into the query
cancellation/cleanup path (the same place that cancels the request) to call
pendingTaskRef(); ensure both the result.then fulfillment/rejection handlers and
the cancellation listener invoke pendingTaskRef() idempotently (guarded so it
only runs once) and that any registered listener is removed after release.
Fixes #10046.
Summary
PendingTasksfor promise-returning query functions as soon as the query function runsquery.datathat is read during render beforewhenStable()@tanstack/angular-query-experimentalValidation
corepack pnpm install --frozen-lockfilecorepack pnpm --filter @tanstack/angular-query-experimental exec vitest run src/__tests__/signal-proxy.test.ts src/__tests__/inject-query.test.ts(2 files, 27 tests passed)corepack pnpm exec eslint packages/angular-query-experimental/src/create-base-query.ts packages/angular-query-experimental/src/__tests__/inject-query.test.tscorepack pnpm exec prettier --check .changeset/angular-query-preaccess-whenstable.md packages/angular-query-experimental/src/create-base-query.ts packages/angular-query-experimental/src/__tests__/inject-query.test.tsgit diff --checkI also ran
corepack pnpm --filter @tanstack/angular-query-experimental exec vitest run; all Angular package tests executed successfully (25 files, 218 tests passed), but the command exited non-zero because this Windows checkout treats repo symlink placeholders likeroot.eslint.config.jsas source and reportsTypeCheckError: Declaration or statement expected.corepack pnpm --filter @tanstack/query-core buildis blocked by the same local symlink placeholder issue inpackages/query-core/root.tsup.config.js.Summary by CodeRabbit
Bug Fixes
whenStable()to remain pending when accessing query result signals before component render completes, ensuring proper synchronization of query state with Angular's rendering lifecycle.Tests
whenStable()remains pending when query result signals are accessed before the component finishes rendering.Chores