Conversation
📝 WalkthroughWalkthroughTest refactored from using Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
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 docstrings
🧪 Generate unit tests (beta)
Comment |
|
View your CI Pipeline Execution ↗ for commit 7d10482
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
size-limit report 📦
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/angular-query-experimental/src/__tests__/inject-is-mutating.test.ts (1)
52-62: Optional: DRY the repeated mutating-text assertions.The three assertions are clear, but a tiny helper would reduce duplication and make future edits safer.
♻️ Suggested cleanup
+ const expectMutating = (value: number) => { + expect(rendered.getByText(`mutating: ${value}`)).toBeInTheDocument() + } + - expect(rendered.getByText('mutating: 0')).toBeInTheDocument() + expectMutating(0) rendered.fixture.componentInstance.mutation.mutate({ par1: 'par1' }) await vi.advanceTimersByTimeAsync(0) rendered.fixture.detectChanges() - expect(rendered.getByText('mutating: 1')).toBeInTheDocument() + expectMutating(1) await vi.advanceTimersByTimeAsync(11) rendered.fixture.detectChanges() - expect(rendered.getByText('mutating: 0')).toBeInTheDocument() + expectMutating(0)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/angular-query-experimental/src/__tests__/inject-is-mutating.test.ts` around lines 52 - 62, The test repeats the same DOM assertion for the mutating count; extract a small helper (e.g., expectMutatingCount or assertMutatingText) near the test in inject-is-mutating.test.ts that accepts the expected number and calls rendered.getByText(`mutating: ${n}`) and toBeInTheDocument(), then replace the three duplicated assertions with calls to that helper around the mutation and timer advances to DRY the test and make future changes safer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/angular-query-experimental/src/__tests__/inject-is-mutating.test.ts`:
- Around line 52-62: The test repeats the same DOM assertion for the mutating
count; extract a small helper (e.g., expectMutatingCount or assertMutatingText)
near the test in inject-is-mutating.test.ts that accepts the expected number and
calls rendered.getByText(`mutating: ${n}`) and toBeInTheDocument(), then replace
the three duplicated assertions with calls to that helper around the mutation
and timer advances to DRY the test and make future changes safer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d6444233-0797-4c48-870b-eda2e00b32c4
📒 Files selected for processing (1)
packages/angular-query-experimental/src/__tests__/inject-is-mutating.test.ts
🎯 Changes
Rewrites the main test in
inject-is-mutating.test.tsto use a@Component+@testing-library/angular'srenderinstead of callingTestBed.runInInjectionContextand asserting on the signal value directly. The assertion now checks the rendered DOM text, which mirrors how a user actually observesinjectIsMutatingin an Angular component.Mirrors the approach taken for
injectIsFetchingin #10556.Before
After
Why
examples/angular/*(readonly mutation = injectMutation(...)inauto-refetching,optimistic-updates, etc.).readonlyfields on the test component follow the convention used in our Angular examples.injection contextdescribe block (NG0203 throw / passing an injector) is kept as-is — those checks don't need a rendered component.Verification
@tanstack/angular-query-experimental— 209 tests passed, 0 type errors✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit