Skip to content

Commit 840b44f

Browse files
ThiloAschebrockThiloAschebrock
committed
fix(angular-query): ensure initial mutation pending state is emitted
1 parent 38b4008 commit 840b44f

File tree

3 files changed

+117
-127
lines changed

3 files changed

+117
-127
lines changed

‎.changeset/puny-melons-deny.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/angular-query-experimental': minor
3+
---
4+
5+
Ensure initial mutation pending state is emitted

‎packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts‎

Lines changed: 39 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ describe('injectMutation', () => {
5555
}))
5656
})
5757

58-
TestBed.tick()
59-
6058
mutation.mutate(result)
6159
await vi.advanceTimersByTimeAsync(0)
6260

@@ -389,11 +387,42 @@ describe('injectMutation', () => {
389387
expect(mutation2!.options.mutationKey).toEqual(['fake', 'updatedValue'])
390388
})
391389

390+
test('should have pending state when mutating in constructor', async () => {
391+
@Component({
392+
selector: 'app-fake',
393+
template: `
394+
<span>{{ mutation.isPending() ? 'pending' : 'not pending' }}</span>
395+
`,
396+
})
397+
class FakeComponent {
398+
mutation = injectMutation(() => ({
399+
mutationKey: ['fake'],
400+
mutationFn: () => sleep(10).then(() => 'fake'),
401+
}))
402+
403+
constructor() {
404+
this.mutation.mutate()
405+
}
406+
}
407+
408+
const fixture = TestBed.createComponent(FakeComponent)
409+
const { debugElement } = fixture
410+
const span = debugElement.query(By.css('span'))
411+
412+
await vi.advanceTimersByTimeAsync(0)
413+
expect(span.nativeElement.textContent).toEqual('pending')
414+
415+
await vi.advanceTimersByTimeAsync(11)
416+
fixture.detectChanges()
417+
418+
expect(span.nativeElement.textContent).toEqual('not pending')
419+
})
420+
392421
describe('throwOnError', () => {
393422
test('should evaluate throwOnError when mutation is expected to throw', async () => {
394423
const err = new Error('Expected mock error. All is well!')
395424
const boundaryFn = vi.fn()
396-
const { mutate } = TestBed.runInInjectionContext(() => {
425+
const { mutate, status, error } = TestBed.runInInjectionContext(() => {
397426
return injectMutation(() => ({
398427
mutationKey: ['fake'],
399428
mutationFn: () => {
@@ -403,14 +432,14 @@ describe('injectMutation', () => {
403432
}))
404433
})
405434

406-
TestBed.tick()
407-
408435
mutate()
409436

410437
await vi.advanceTimersByTimeAsync(0)
411438

412439
expect(boundaryFn).toHaveBeenCalledTimes(1)
413440
expect(boundaryFn).toHaveBeenCalledWith(err)
441+
expect(status()).toBe('error')
442+
expect(error()).toBe(err)
414443
})
415444
})
416445

@@ -533,21 +562,8 @@ describe('injectMutation', () => {
533562
// Start mutation
534563
mutation.mutate('retry-test')
535564

536-
// Synchronize pending effects for each retry attempt
537-
TestBed.tick()
538-
await Promise.resolve()
539-
await vi.advanceTimersByTimeAsync(10)
540-
541-
TestBed.tick()
542-
await Promise.resolve()
543-
await vi.advanceTimersByTimeAsync(10)
544-
545-
TestBed.tick()
546-
547-
const stablePromise = app.whenStable()
548-
await Promise.resolve()
549-
await vi.advanceTimersByTimeAsync(10)
550-
await stablePromise
565+
await vi.advanceTimersByTimeAsync(30)
566+
await app.whenStable()
551567

552568
expect(mutation.isSuccess()).toBe(true)
553569
expect(mutation.data()).toBe('processed: retry-test')
@@ -590,14 +606,8 @@ describe('injectMutation', () => {
590606
mutation1.mutate('test1')
591607
mutation2.mutate('test2')
592608

593-
// Synchronize pending effects
594-
TestBed.tick()
595-
596-
const stablePromise = app.whenStable()
597-
// Flush microtasks to allow TanStack Query's scheduled notifications to process
598-
await Promise.resolve()
599609
await vi.advanceTimersByTimeAsync(1)
600-
await stablePromise
610+
await app.whenStable()
601611

602612
expect(mutation1.isSuccess()).toBe(true)
603613
expect(mutation1.data()).toBe('mutation1: test1')
@@ -642,14 +652,8 @@ describe('injectMutation', () => {
642652
// Start mutation
643653
mutation.mutate('test')
644654

645-
// Synchronize pending effects
646-
TestBed.tick()
647-
648-
const stablePromise = app.whenStable()
649-
// Flush microtasks to allow TanStack Query's scheduled notifications to process
650-
await Promise.resolve()
651655
await vi.advanceTimersByTimeAsync(1)
652-
await stablePromise
656+
await app.whenStable()
653657

654658
expect(onMutateCalled).toBe(true)
655659
expect(onSuccessCalled).toBe(true)
@@ -679,14 +683,8 @@ describe('injectMutation', () => {
679683
// Start mutation
680684
mutation.mutate('test')
681685

682-
// Synchronize pending effects
683-
TestBed.tick()
684-
685-
const stablePromise = app.whenStable()
686-
// Flush microtasks to allow TanStack Query's scheduled notifications to process
687-
await Promise.resolve()
688686
await vi.advanceTimersByTimeAsync(1)
689-
await stablePromise
687+
await app.whenStable()
690688

691689
// Synchronous mutations complete immediately
692690
expect(mutation.isSuccess()).toBe(true)

‎packages/angular-query-experimental/src/inject-mutation.ts‎

Lines changed: 73 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import {
2+
DestroyRef,
23
Injector,
34
NgZone,
45
assertInInjectionContext,
56
computed,
6-
effect,
77
inject,
88
signal,
99
untracked,
@@ -17,8 +17,7 @@ import {
1717
} from '@tanstack/query-core'
1818
import { signalProxy } from './signal-proxy'
1919
import { PENDING_TASKS } from './pending-tasks-compat'
20-
import type { PendingTaskRef } from './pending-tasks-compat'
21-
import type { DefaultError, MutationObserverResult } from '@tanstack/query-core'
20+
import type { DefaultError } from '@tanstack/query-core'
2221
import type {
2322
CreateMutateFunction,
2423
CreateMutationOptions,
@@ -58,6 +57,7 @@ export function injectMutation<
5857
): CreateMutationResult<TData, TError, TVariables, TOnMutateResult> {
5958
!options?.injector && assertInInjectionContext(injectMutation)
6059
const injector = options?.injector ?? inject(Injector)
60+
const destroyRef = injector.get(DestroyRef)
6161
const ngZone = injector.get(NgZone)
6262
const pendingTasks = injector.get(PENDING_TASKS)
6363
const queryClient = injector.get(QueryClient)
@@ -78,7 +78,15 @@ export function injectMutation<
7878
> | null = null
7979

8080
return computed(() => {
81-
return (instance ||= new MutationObserver(queryClient, optionsSignal()))
81+
const observerOptions = optionsSignal()
82+
return untracked(() => {
83+
if (instance) {
84+
instance.setOptions(observerOptions)
85+
} else {
86+
instance = new MutationObserver(queryClient, observerOptions)
87+
}
88+
return instance
89+
})
8290
})
8391
})()
8492

@@ -87,97 +95,74 @@ export function injectMutation<
8795
>(() => {
8896
const observer = observerSignal()
8997
return (variables, mutateOptions) => {
90-
observer.mutate(variables, mutateOptions).catch(noop)
98+
void observer.mutate(variables, mutateOptions).catch(noop)
9199
}
92100
})
93101

94-
/**
95-
* Computed signal that gets result from mutation cache based on passed options
96-
*/
97-
const resultFromInitialOptionsSignal = computed(() => {
98-
const observer = observerSignal()
99-
return observer.getCurrentResult()
100-
})
102+
let cleanup: () => void = noop
101103

102104
/**
103-
* Signal that contains result set by subscriber
105+
* Returning a writable signal from a computed is similar to `linkedSignal`,
106+
* but compatible with Angular < 19
107+
*
108+
* Compared to `linkedSignal`, this pattern requires extra parentheses:
109+
* - Accessing value: `result()()`
110+
* - Setting value: `result().set(newValue)`
104111
*/
105-
const resultFromSubscriberSignal = signal<MutationObserverResult<
106-
TData,
107-
TError,
108-
TVariables,
109-
TOnMutateResult
110-
> | null>(null)
111-
112-
effect(
113-
() => {
114-
const observer = observerSignal()
115-
const observerOptions = optionsSignal()
112+
const linkedResultSignal = computed(() => {
113+
const observer = observerSignal()
116114

117-
untracked(() => {
118-
observer.setOptions(observerOptions)
119-
})
120-
},
121-
{
122-
injector,
123-
},
124-
)
125-
126-
effect(
127-
(onCleanup) => {
128-
// observer.trackResult is not used as this optimization is not needed for Angular
129-
const observer = observerSignal()
130-
let pendingTaskRef: PendingTaskRef | null = null
131-
132-
untracked(() => {
133-
const unsubscribe = ngZone.runOutsideAngular(() =>
134-
observer.subscribe(
135-
notifyManager.batchCalls((state) => {
136-
ngZone.run(() => {
137-
// Track pending task when mutation is pending
138-
if (state.isPending && !pendingTaskRef) {
139-
pendingTaskRef = pendingTasks.add()
140-
}
141-
142-
// Clear pending task when mutation is no longer pending
143-
if (!state.isPending && pendingTaskRef) {
144-
pendingTaskRef()
145-
pendingTaskRef = null
146-
}
147-
148-
if (
149-
state.isError &&
150-
shouldThrowError(observer.options.throwOnError, [state.error])
151-
) {
152-
ngZone.onError.emit(state.error)
153-
throw state.error
154-
}
155-
156-
resultFromSubscriberSignal.set(state)
157-
})
158-
}),
159-
),
160-
)
161-
onCleanup(() => {
162-
// Clean up any pending task on destroy
163-
if (pendingTaskRef) {
164-
pendingTaskRef()
165-
pendingTaskRef = null
166-
}
167-
unsubscribe()
168-
})
169-
})
170-
},
171-
{
172-
injector,
173-
},
174-
)
115+
return untracked(() => {
116+
const currentResult = observer.getCurrentResult()
117+
const result = signal(currentResult)
118+
119+
cleanup()
120+
let pendingTaskRef = currentResult.isPending ? pendingTasks.add() : null
121+
122+
const unsubscribe = ngZone.runOutsideAngular(() =>
123+
observer.subscribe(
124+
notifyManager.batchCalls((state) => {
125+
ngZone.run(() => {
126+
result.set(state)
127+
128+
// Track pending task when mutation is pending
129+
if (state.isPending && !pendingTaskRef) {
130+
pendingTaskRef = pendingTasks.add()
131+
}
132+
133+
// Clear pending task when mutation is no longer pending
134+
if (!state.isPending && pendingTaskRef) {
135+
pendingTaskRef()
136+
pendingTaskRef = null
137+
}
138+
139+
if (
140+
state.isError &&
141+
shouldThrowError(observer.options.throwOnError, [state.error])
142+
) {
143+
ngZone.onError.emit(state.error)
144+
throw state.error
145+
}
146+
})
147+
}),
148+
),
149+
)
150+
151+
cleanup = () => {
152+
// Clean up any pending task on destroy
153+
if (pendingTaskRef) {
154+
pendingTaskRef()
155+
pendingTaskRef = null
156+
}
157+
unsubscribe()
158+
}
159+
160+
return result
161+
})
162+
})
175163

176164
const resultSignal = computed(() => {
177-
const resultFromSubscriber = resultFromSubscriberSignal()
178-
const resultFromInitialOptions = resultFromInitialOptionsSignal()
179-
180-
const result = resultFromSubscriber ?? resultFromInitialOptions
165+
const result = linkedResultSignal()()
181166

182167
return {
183168
...result,
@@ -186,6 +171,8 @@ export function injectMutation<
186171
}
187172
})
188173

174+
destroyRef.onDestroy(() => cleanup())
175+
189176
return signalProxy(resultSignal) as CreateMutationResult<
190177
TData,
191178
TError,

0 commit comments

Comments
 (0)