Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(angular-query): subscribe to observer outside the Angular zone #8331

Merged
merged 1 commit into from
Nov 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 21 additions & 17 deletions packages/angular-query-experimental/src/create-base-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,23 +88,27 @@ export function createBaseQuery<
)

// observer.trackResult is not used as this optimization is not needed for Angular
const unsubscribe = observer.subscribe(
notifyManager.batchCalls((state: QueryObserverResult<TData, TError>) => {
ngZone.run(() => {
if (
state.isError &&
!state.isFetching &&
// !isRestoring() && // todo: enable when client persistence is implemented
shouldThrowError(observer.options.throwOnError, [
state.error,
observer.getCurrentQuery(),
])
) {
throw state.error
}
resultSignal.set(state)
})
}),
const unsubscribe = ngZone.runOutsideAngular(() =>
observer.subscribe(
notifyManager.batchCalls(
(state: QueryObserverResult<TData, TError>) => {
ngZone.run(() => {
if (
state.isError &&
!state.isFetching &&
// !isRestoring() && // todo: enable when client persistence is implemented
shouldThrowError(observer.options.throwOnError, [
state.error,
observer.getCurrentQuery(),
])
) {
throw state.error
}
resultSignal.set(state)
})
},
),
),
)
destroyRef.onDestroy(unsubscribe)

Expand Down
24 changes: 13 additions & 11 deletions packages/angular-query-experimental/src/inject-is-fetching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,19 @@ export function injectIsFetching(

const result = signal(isFetching)

const unsubscribe = cache.subscribe(
notifyManager.batchCalls(() => {
const newIsFetching = queryClient.isFetching(filters)
if (isFetching !== newIsFetching) {
// * and update with each change
isFetching = newIsFetching
ngZone.run(() => {
result.set(isFetching)
})
}
}),
const unsubscribe = ngZone.runOutsideAngular(() =>
cache.subscribe(
notifyManager.batchCalls(() => {
const newIsFetching = queryClient.isFetching(filters)
if (isFetching !== newIsFetching) {
// * and update with each change
isFetching = newIsFetching
ngZone.run(() => {
result.set(isFetching)
})
}
}),
),
)

destroyRef.onDestroy(unsubscribe)
Expand Down
24 changes: 13 additions & 11 deletions packages/angular-query-experimental/src/inject-is-mutating.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,19 @@ export function injectIsMutating(

const result = signal(isMutating)

const unsubscribe = cache.subscribe(
notifyManager.batchCalls(() => {
const newIsMutating = queryClient.isMutating(filters)
if (isMutating !== newIsMutating) {
// * and update with each change
isMutating = newIsMutating
ngZone.run(() => {
result.set(isMutating)
})
}
}),
const unsubscribe = ngZone.runOutsideAngular(() =>
cache.subscribe(
notifyManager.batchCalls(() => {
const newIsMutating = queryClient.isMutating(filters)
if (isMutating !== newIsMutating) {
// * and update with each change
isMutating = newIsMutating
ngZone.run(() => {
result.set(isMutating)
})
}
}),
),
)

destroyRef.onDestroy(unsubscribe)
Expand Down
26 changes: 14 additions & 12 deletions packages/angular-query-experimental/src/inject-mutation-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,20 @@ export function injectMutationState<TResult = MutationState>(
{ injector },
)

const unsubscribe = mutationCache.subscribe(
notifyManager.batchCalls(() => {
const nextResult = replaceEqualDeep(
result(),
getResult(mutationCache, mutationStateOptionsFn()),
)
if (result() !== nextResult) {
ngZone.run(() => {
result.set(nextResult)
})
}
}),
const unsubscribe = ngZone.runOutsideAngular(() =>
mutationCache.subscribe(
notifyManager.batchCalls(() => {
const nextResult = replaceEqualDeep(
result(),
getResult(mutationCache, mutationStateOptionsFn()),
)
if (result() !== nextResult) {
ngZone.run(() => {
result.set(nextResult)
})
}
}),
),
)

destroyRef.onDestroy(unsubscribe)
Expand Down
44 changes: 24 additions & 20 deletions packages/angular-query-experimental/src/inject-mutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,26 +71,30 @@

const result = signal(observer.getCurrentResult())

const unsubscribe = observer.subscribe(
notifyManager.batchCalls(
(
state: MutationObserverResult<
TData,
TError,
TVariables,
TContext
>,
) => {
ngZone.run(() => {
if (
state.isError &&
shouldThrowError(observer.options.throwOnError, [state.error])
) {
throw state.error
}
result.set(state)
})
},
const unsubscribe = ngZone.runOutsideAngular(() =>
observer.subscribe(
notifyManager.batchCalls(
(
state: MutationObserverResult<
TData,
TError,
TVariables,
TContext
>,
) => {
ngZone.run(() => {
if (
state.isError &&
shouldThrowError(observer.options.throwOnError, [
state.error,
])
) {
throw state.error

Check warning on line 92 in packages/angular-query-experimental/src/inject-mutation.ts

View check run for this annotation

Codecov / codecov/patch

packages/angular-query-experimental/src/inject-mutation.ts#L92

Added line #L92 was not covered by tests
}
result.set(state)
})
},
),
),
)

Expand Down
16 changes: 13 additions & 3 deletions packages/angular-query-experimental/src/inject-queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@
QueryClient,
notifyManager,
} from '@tanstack/query-core'
import { DestroyRef, computed, effect, inject, signal } from '@angular/core'
import {
DestroyRef,
NgZone,
computed,
effect,
inject,
signal,
} from '@angular/core'
import { assertInjector } from './util/assert-injector/assert-injector'
import type { Injector, Signal } from '@angular/core'
import type {
Expand Down Expand Up @@ -202,8 +209,9 @@
injector?: Injector,
): Signal<TCombinedResult> {
return assertInjector(injectQueries, injector, () => {
const queryClient = inject(QueryClient)
const destroyRef = inject(DestroyRef)
const ngZone = inject(NgZone)
const queryClient = inject(QueryClient)

Check warning on line 214 in packages/angular-query-experimental/src/inject-queries.ts

View check run for this annotation

Codecov / codecov/patch

packages/angular-query-experimental/src/inject-queries.ts#L213-L214

Added lines #L213 - L214 were not covered by tests

const defaultedQueries = computed(() => {
return queries().map((opts) => {
Expand Down Expand Up @@ -238,7 +246,9 @@

const result = signal(getCombinedResult() as any)

const unsubscribe = observer.subscribe(notifyManager.batchCalls(result.set))
const unsubscribe = ngZone.runOutsideAngular(() =>
observer.subscribe(notifyManager.batchCalls(result.set)),

Check warning on line 250 in packages/angular-query-experimental/src/inject-queries.ts

View check run for this annotation

Codecov / codecov/patch

packages/angular-query-experimental/src/inject-queries.ts#L249-L250

Added lines #L249 - L250 were not covered by tests
)
destroyRef.onDestroy(unsubscribe)

return result
Expand Down
Loading