Skip to content

Commit dfa8c31

Browse files
committed
Use queueMicrotask instead of notifyManager.batchCalls to improve timing
1 parent a2982d5 commit dfa8c31

File tree

3 files changed

+78
-38
lines changed

3 files changed

+78
-38
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
provideZonelessChangeDetection,
88
} from '@angular/core'
99
import { sleep } from '@tanstack/query-test-utils'
10-
import { injectInfiniteQuery, provideTanStackQuery, QueryClient } from '..'
10+
import { QueryClient, injectInfiniteQuery, provideTanStackQuery } from '..'
1111
import { expectSignals } from './test-utils'
1212

1313
describe('injectInfiniteQuery', () => {

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

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
import {
22
ApplicationRef,
3+
ChangeDetectionStrategy,
34
Component,
45
Injector,
6+
NgZone,
57
computed,
68
input,
79
provideZonelessChangeDetection,
810
signal,
9-
ChangeDetectionStrategy,
1011
} from '@angular/core'
1112
import { TestBed } from '@angular/core/testing'
1213
import { HttpClient, provideHttpClient } from '@angular/common/http'
@@ -554,6 +555,14 @@ describe('injectQuery', () => {
554555
})
555556

556557
test('should throw when throwOnError is true', async () => {
558+
const zone = TestBed.inject(NgZone)
559+
const errorPromise = new Promise<Error>((resolve) => {
560+
const sub = zone.onError.subscribe((error) => {
561+
sub.unsubscribe()
562+
resolve(error as Error)
563+
})
564+
})
565+
557566
@Component({
558567
selector: 'app-test',
559568
template: '',
@@ -571,10 +580,19 @@ describe('injectQuery', () => {
571580

572581
TestBed.createComponent(TestComponent).detectChanges()
573582

574-
await expect(vi.runAllTimersAsync()).rejects.toThrow('Some error')
583+
await vi.runAllTimersAsync()
584+
await expect(errorPromise).resolves.toEqual(Error('Some error'))
575585
})
576586

577587
test('should throw when throwOnError function returns true', async () => {
588+
const zone = TestBed.inject(NgZone)
589+
const errorPromise = new Promise<Error>((resolve) => {
590+
const sub = zone.onError.subscribe((error) => {
591+
sub.unsubscribe()
592+
resolve(error as Error)
593+
})
594+
})
595+
578596
@Component({
579597
selector: 'app-test',
580598
template: '',
@@ -592,7 +610,8 @@ describe('injectQuery', () => {
592610

593611
TestBed.createComponent(TestComponent).detectChanges()
594612

595-
await expect(vi.runAllTimersAsync()).rejects.toThrow('Some error')
613+
await vi.runAllTimersAsync()
614+
await expect(errorPromise).resolves.toEqual(Error('Some error'))
596615
})
597616
})
598617

@@ -715,9 +734,8 @@ describe('injectQuery', () => {
715734
expect(query.status()).toBe('pending')
716735
expect(query.data()).toBeUndefined()
717736

718-
const stablePromise = app.whenStable()
719737
await vi.advanceTimersByTimeAsync(60)
720-
await stablePromise
738+
await app.whenStable()
721739

722740
expect(query.status()).toBe('success')
723741
expect(query.data()).toBe('test data')
@@ -767,9 +785,8 @@ describe('injectQuery', () => {
767785
expect(query.status()).toBe('pending')
768786

769787
// Advance timers and wait for Angular to be "stable"
770-
const stablePromise = app.whenStable()
771788
await vi.advanceTimersByTimeAsync(20)
772-
await stablePromise
789+
await app.whenStable()
773790

774791
// Query should be complete after whenStable() thanks to PendingTasks integration
775792
expect(query.status()).toBe('success')
@@ -865,6 +882,7 @@ describe('injectQuery', () => {
865882
const query = component.query
866883

867884
// Initially disabled
885+
await vi.advanceTimersByTimeAsync(0)
868886
await app.whenStable()
869887
expect(query.status()).toBe('pending')
870888
expect(query.data()).toBeUndefined()
@@ -874,6 +892,7 @@ describe('injectQuery', () => {
874892
enabledSignal.set(true)
875893
fixture.detectChanges()
876894

895+
await vi.advanceTimersByTimeAsync(0)
877896
await app.whenStable()
878897
expect(query.status()).toBe('success')
879898
expect(query.data()).toBe('sync-data-1')

packages/angular-query-experimental/src/create-base-query.ts

Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
1-
import { DestroyRef, NgZone, PendingTasks, computed, effect, inject, linkedSignal, untracked, } from '@angular/core'
2-
import { QueryClient, notifyManager, shouldThrowError, } from '@tanstack/query-core'
1+
import {
2+
DestroyRef,
3+
NgZone,
4+
PendingTasks,
5+
computed,
6+
effect,
7+
inject,
8+
linkedSignal,
9+
untracked,
10+
} from '@angular/core'
11+
import {
12+
QueryClient,
13+
notifyManager,
14+
shouldThrowError,
15+
} from '@tanstack/query-core'
316
import { signalProxy } from './signal-proxy'
417
import { injectIsRestoring } from './inject-is-restoring'
518
import type {
@@ -45,6 +58,7 @@ export function createBaseQuery<
4558
TQueryKey
4659
> | null = null
4760

61+
let destroyed = false
4862
let taskCleanupRef: (() => void) | null = null
4963

5064
const startPendingTask = () => {
@@ -127,35 +141,39 @@ export function createBaseQuery<
127141

128142
observer = new Observer(queryClient, options)
129143

130-
const unsubscribe = observer.subscribe(
131-
notifyManager.batchCalls((state) => {
132-
ngZone.run(() => {
133-
if (state.fetchStatus !== 'idle') {
134-
startPendingTask()
135-
} else {
136-
stopPendingTask()
137-
}
138-
139-
if (
140-
state.isError &&
141-
!state.isFetching &&
142-
shouldThrowError(observer!.options.throwOnError, [
143-
state.error,
144-
observer!.getCurrentQuery(),
145-
])
146-
) {
147-
ngZone.onError.emit(state.error)
148-
throw state.error
149-
}
150-
const trackedState = trackObserverResult(
151-
state,
152-
observer!.options.notifyOnChangeProps,
153-
)
154-
resultSignal.set(trackedState)
144+
const unsubscribe = observer.subscribe((state) => {
145+
if (state.fetchStatus !== 'idle') {
146+
startPendingTask()
147+
} else {
148+
stopPendingTask()
149+
}
150+
151+
queueMicrotask(() => {
152+
if (destroyed) return
153+
notifyManager.batch(() => {
154+
ngZone.run(() => {
155+
if (
156+
state.isError &&
157+
!state.isFetching &&
158+
shouldThrowError(observer!.options.throwOnError, [
159+
state.error,
160+
observer!.getCurrentQuery(),
161+
])
162+
) {
163+
ngZone.onError.emit(state.error)
164+
throw state.error
165+
}
166+
const trackedState = trackObserverResult(
167+
state,
168+
observer!.options.notifyOnChangeProps,
169+
)
170+
resultSignal.set(trackedState)
171+
})
155172
})
156-
}),
157-
)
173+
})
174+
})
158175
destroyRef.onDestroy(() => {
176+
destroyed = true
159177
unsubscribe()
160178
stopPendingTask()
161179
})
@@ -164,7 +182,10 @@ export function createBaseQuery<
164182
const resultSignal = linkedSignal({
165183
source: defaultedOptionsSignal,
166184
computation: () => {
167-
if (!observer) throw new Error('Observer is not initialized')
185+
if (!observer)
186+
throw new Error(
187+
'injectQuery: QueryObserver not initialized yet. Avoid reading the query result during construction',
188+
)
168189
const defaultedOptions = defaultedOptionsSignal()
169190
const result = observer.getOptimisticResult(defaultedOptions)
170191
return trackObserverResult(result, defaultedOptions.notifyOnChangeProps)

0 commit comments

Comments
 (0)