Skip to content

fix(angular-query): do not run callbacks in injection context #8817

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

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@ export function injectDevtoolsPanel(
const currentInjector = injector ?? inject(Injector)

return runInInjectionContext(currentInjector, () => {
const destroyRef = inject(DestroyRef)
const isBrowser = isPlatformBrowser(inject(PLATFORM_ID))
const injectedClient = inject(QueryClient, { optional: true })

const options = computed(optionsFn)
let devtools: TanstackQueryDevtoolsPanel | null = null

const isBrowser = isPlatformBrowser(inject(PLATFORM_ID))

const destroy = () => {
devtools?.unmount()
devtools = null
Expand All @@ -53,10 +55,7 @@ export function injectDevtoolsPanel(
destroy,
}

const destroyRef = inject(DestroyRef)

effect(() => {
const injectedClient = currentInjector.get(QueryClient, null)
const {
client = injectedClient,
errorTypes = [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ describe('injectMutationState', () => {
@Component({
selector: 'app-fake',
template: `
@for (mutation of mutationState(); track mutation) {
@for (mutation of mutationState(); track $index) {
<span>{{ mutation.status }}</span>
}
`,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import {
Component,
Injectable,
Injector,
inject,
input,
provideExperimentalZonelessChangeDetection,
signal,
Expand Down Expand Up @@ -451,47 +449,6 @@ describe('injectMutation', () => {
await expect(() => mutateAsync()).rejects.toThrowError(err)
})

test('should execute callback in injection context', async () => {
const errorSpy = vi.fn()
@Injectable()
class FakeService {
updateData(name: string) {
return Promise.resolve(name)
}
}

@Component({
selector: 'app-fake',
template: ``,
standalone: true,
providers: [FakeService],
})
class FakeComponent {
mutation = injectMutation(() => {
try {
const service = inject(FakeService)
return {
mutationFn: (name: string) => service.updateData(name),
}
} catch (e) {
errorSpy(e)
throw e
}
})
}

const fixture = TestBed.createComponent(FakeComponent)
fixture.detectChanges()

// check if injection contexts persist in a different task
await new Promise<void>((resolve) => queueMicrotask(() => resolve()))

expect(
await fixture.componentInstance.mutation.mutateAsync('test'),
).toEqual('test')
expect(errorSpy).not.toHaveBeenCalled()
})

describe('injection context', () => {
test('throws NG0203 with descriptive error outside injection context', () => {
expect(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import {
Component,
Injectable,
Injector,
computed,
effect,
inject,
input,
provideExperimentalZonelessChangeDetection,
signal,
Expand Down Expand Up @@ -537,91 +535,6 @@ describe('injectQuery', () => {
)
})

test('should run optionsFn in injection context', async () => {
@Injectable()
class FakeService {
getData(name: string) {
return Promise.resolve(name)
}
}

@Component({
selector: 'app-fake',
template: `{{ query.data() }}`,
standalone: true,
providers: [FakeService],
})
class FakeComponent {
name = signal<string>('test name')

query = injectQuery(() => {
const service = inject(FakeService)

return {
queryKey: ['fake', this.name()],
queryFn: () => {
return service.getData(this.name())
},
}
})
}

const fixture = TestBed.createComponent(FakeComponent)
fixture.detectChanges()
await resolveQueries()

expect(fixture.componentInstance.query.data()).toEqual('test name')

fixture.componentInstance.name.set('test name 2')
fixture.detectChanges()
await resolveQueries()

expect(fixture.componentInstance.query.data()).toEqual('test name 2')
})

test('should run optionsFn in injection context and allow passing injector to queryFn', async () => {
@Injectable()
class FakeService {
getData(name: string) {
return Promise.resolve(name)
}
}

@Component({
selector: 'app-fake',
template: `{{ query.data() }}`,
standalone: true,
providers: [FakeService],
})
class FakeComponent {
name = signal<string>('test name')

query = injectQuery(() => {
const injector = inject(Injector)

return {
queryKey: ['fake', this.name()],
queryFn: () => {
const service = injector.get(FakeService)
return service.getData(this.name())
},
}
})
}

const fixture = TestBed.createComponent(FakeComponent)
fixture.detectChanges()
await resolveQueries()

expect(fixture.componentInstance.query.data()).toEqual('test name')

fixture.componentInstance.name.set('test name 2')
fixture.detectChanges()
await resolveQueries()

expect(fixture.componentInstance.query.data()).toEqual('test name 2')
})

describe('injection context', () => {
test('throws NG0203 with descriptive error outside injection context', () => {
expect(() => {
Expand Down
13 changes: 4 additions & 9 deletions packages/angular-query-experimental/src/create-base-query.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import {
DestroyRef,
Injector,
NgZone,
VERSION,
computed,
effect,
inject,
runInInjectionContext,
signal,
untracked,
} from '@angular/core'
Expand Down Expand Up @@ -39,10 +37,9 @@ export function createBaseQuery<
>,
Observer: typeof QueryObserver,
) {
const injector = inject(Injector)
const ngZone = injector.get(NgZone)
const destroyRef = injector.get(DestroyRef)
const queryClient = injector.get(QueryClient)
const ngZone = inject(NgZone)
const destroyRef = inject(DestroyRef)
const queryClient = inject(QueryClient)

/**
* Signal that has the default options from query client applied
Expand All @@ -51,8 +48,7 @@ export function createBaseQuery<
* are preserved and can keep being applied after signal changes
*/
const defaultedOptionsSignal = computed(() => {
const options = runInInjectionContext(injector, () => optionsFn())
const defaultedOptions = queryClient.defaultQueryOptions(options)
const defaultedOptions = queryClient.defaultQueryOptions(optionsFn())
defaultedOptions._optimisticResults = 'optimistic'
return defaultedOptions
})
Expand Down Expand Up @@ -100,7 +96,6 @@ export function createBaseQuery<
// Set allowSignalWrites to support Angular < v19
// Set to undefined to avoid warning on newer versions
allowSignalWrites: VERSION.major < '19' || undefined,
injector,
},
)

Expand Down
8 changes: 2 additions & 6 deletions packages/angular-query-experimental/src/inject-mutation.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import {
DestroyRef,
Injector,
NgZone,
computed,
effect,
inject,
runInInjectionContext,
signal,
untracked,
} from '@angular/core'
Expand All @@ -17,6 +15,7 @@ import {
import { assertInjector } from './util/assert-injector/assert-injector'
import { signalProxy } from './signal-proxy'
import { noop, shouldThrowError } from './util'
import type { Injector } from '@angular/core'
import type { DefaultError, MutationObserverResult } from '@tanstack/query-core'
import type { CreateMutateFunction, CreateMutationResult } from './types'
import type { CreateMutationOptions } from './mutation-options'
Expand All @@ -40,7 +39,6 @@ export function injectMutation<
injector?: Injector,
): CreateMutationResult<TData, TError, TVariables, TContext> {
return assertInjector(injectMutation, injector, () => {
const currentInjector = inject(Injector)
const destroyRef = inject(DestroyRef)
const ngZone = inject(NgZone)
const queryClient = inject(QueryClient)
Expand All @@ -50,9 +48,7 @@ export function injectMutation<
* making it reactive. Wrapping options in a function ensures embedded expressions
* are preserved and can keep being applied after signal changes
*/
const optionsSignal = computed(() =>
runInInjectionContext(currentInjector, () => optionsFn()),
)
const optionsSignal = computed(optionsFn)

const observerSignal = (() => {
let instance: MutationObserver<
Expand Down
44 changes: 20 additions & 24 deletions packages/angular-query-experimental/src/providers.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import {
DestroyRef,
ENVIRONMENT_INITIALIZER,
Injector,
PLATFORM_ID,
computed,
effect,
inject,
makeEnvironmentProviders,
runInInjectionContext,
} from '@angular/core'
import { QueryClient, onlineManager } from '@tanstack/query-core'
import { isPlatformBrowser } from '@angular/common'
Expand Down Expand Up @@ -99,7 +97,7 @@ export function provideTanStackQuery(
return makeEnvironmentProviders([
provideQueryClient(queryClient),
{
// Do not use provideEnvironmentInitializer to support Angular < v19
// Do not use provideEnvironmentInitializer while Angular < v19 is supported
provide: ENVIRONMENT_INITIALIZER,
multi: true,
useValue: () => {
Expand Down Expand Up @@ -250,14 +248,17 @@ export function withDevtools(
} else {
providers = [
{
// Do not use provideEnvironmentInitializer while Angular < v19 is supported
provide: ENVIRONMENT_INITIALIZER,
multi: true,
useFactory: () => {
if (!isPlatformBrowser(inject(PLATFORM_ID))) return noop
const injector = inject(Injector)
const options = computed(() =>
runInInjectionContext(injector, () => optionsFn?.() ?? {}),
)
const injectedClient = inject(QueryClient, {
optional: true,
})
const destroyRef = inject(DestroyRef)

const options = computed(() => optionsFn?.() ?? {})

let devtools: TanstackQueryDevtools | null = null
let el: HTMLElement | null = null
Expand All @@ -269,10 +270,7 @@ export function withDevtools(
: isDevMode()
})

const destroyRef = inject(DestroyRef)

const getResolvedQueryClient = () => {
const injectedClient = injector.get(QueryClient, null)
const client = options().client ?? injectedClient
if (!client) {
throw new Error('No QueryClient found')
Expand Down Expand Up @@ -314,22 +312,20 @@ export function withDevtools(
el = document.body.appendChild(document.createElement('div'))
el.classList.add('tsqd-parent-container')

import('@tanstack/query-devtools').then((queryDevtools) =>
runInInjectionContext(injector, () => {
devtools = new queryDevtools.TanstackQueryDevtools({
...options(),
client: getResolvedQueryClient(),
queryFlavor: 'Angular Query',
version: '5',
onlineManager,
})
import('@tanstack/query-devtools').then((queryDevtools) => {
devtools = new queryDevtools.TanstackQueryDevtools({
...options(),
client: getResolvedQueryClient(),
queryFlavor: 'Angular Query',
version: '5',
onlineManager,
})

el && devtools.mount(el)
el && devtools.mount(el)

// Unmount the devtools on application destroy
destroyRef.onDestroy(destroyDevtools)
}),
)
// Unmount the devtools on application destroy
destroyRef.onDestroy(destroyDevtools)
})
})
},
},
Expand Down
Loading