Skip to content

fix(angular-query): track pre-render query promises#10779

Open
raashish1601 wants to merge 1 commit into
TanStack:mainfrom
raashish1601:fix/angular-query-whenstable-preaccess
Open

fix(angular-query): track pre-render query promises#10779
raashish1601 wants to merge 1 commit into
TanStack:mainfrom
raashish1601:fix/angular-query-whenstable-preaccess

Conversation

@raashish1601
Copy link
Copy Markdown
Contributor

@raashish1601 raashish1601 commented May 24, 2026

Fixes #10046.

Summary

  • register Angular PendingTasks for promise-returning query functions as soon as the query function runs
  • preserves synchronous query functions without adding an extra pending task
  • adds a regression for a class-field alias of query.data that is read during render before whenStable()
  • adds a patch changeset for @tanstack/angular-query-experimental

Validation

  • corepack pnpm install --frozen-lockfile
  • corepack 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.ts
  • corepack 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.ts
  • git diff --check

I 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 like root.eslint.config.js as source and reports TypeCheckError: Declaration or statement expected.

corepack pnpm --filter @tanstack/query-core build is blocked by the same local symlink placeholder issue in packages/query-core/root.tsup.config.js.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed 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

    • Added test case verifying that whenStable() remains pending when query result signals are accessed before the component finishes rendering.
  • Chores

    • Added release changeset.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 24, 2026

📝 Walkthrough

Walkthrough

This PR fixes a bug where accessing a query's data signal before initial render causes Angular's whenStable() to resolve prematurely. The fix wraps the query function to register pending tasks with Angular's zone and refactors state updates to manage the task lifecycle. A test verifies the fix works correctly.

Changes

Keep Angular whenStable() pending when query data accessed early

Layer / File(s) Summary
Pending task tracking in query resolution
packages/angular-query-experimental/src/create-base-query.ts
Query function is wrapped to register and unregister pending tasks with PENDING_TASKS when the query function returns a thenable. State-update logic is refactored into a new updateState function that manages the pending task lifecycle based on fetch status, handles conditional error emission and throwing, and updates the result signal via notifyManager.batchCalls within the Angular zone.
Test verification and release metadata
packages/angular-query-experimental/src/__tests__/inject-query.test.ts, .changeset/angular-query-preaccess-whenstable.md
New test verifies that when a query data signal is captured before render, fixture.whenStable() remains pending until the async query resolves and both component and query data reflect the resolved result. Changeset documents the patch behavior change.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • TkDodo
  • chughgaurav

Poem

A query once danced before the zone knew,
Signals so eager, whenStable() slipped through,
But now pending tasks keep the stage in its place,
Till async completes at the proper pace— 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: tracking pre-render query promises to fix the Angular whenStable() issue.
Description check ✅ Passed The PR description includes clear summary of changes, validation steps, and acknowledges known local issues, meeting template requirements.
Linked Issues check ✅ Passed Changes directly address #10046 by registering PendingTasks for promise-returning query functions, preventing undefined data when query.data is accessed before render.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the whenStable() issue: a changeset, regression test, and core implementation changes to track pending tasks.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 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

📥 Commits

Reviewing files that changed from the base of the PR and between 63b3a72 and 79a8022.

📒 Files selected for processing (3)
  • .changeset/angular-query-preaccess-whenstable.md
  • packages/angular-query-experimental/src/__tests__/inject-query.test.ts
  • packages/angular-query-experimental/src/create-base-query.ts

Comment on lines +70 to +75
if (result && typeof result.then === 'function') {
const pendingTaskRef = pendingTasks.add()
void result.then(
() => pendingTaskRef(),
() => pendingTaskRef(),
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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.ts

Repository: 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Weird bug with Angular unit tests

1 participant