From a0503de591b6949a4e58c4c3bd001f5e78fbbc6f Mon Sep 17 00:00:00 2001 From: Arnoud de Vries <6420061+arnoud-dv@users.noreply.github.com> Date: Sun, 16 Mar 2025 22:32:44 +0100 Subject: [PATCH] fix(angular-query): do not run callbacks in injection context --- .../src/inject-devtools-panel.ts | 9 +- .../__tests__/inject-mutation-state.test.ts | 2 +- .../src/__tests__/inject-mutation.test.ts | 43 --------- .../src/__tests__/inject-query.test.ts | 87 ------------------- .../src/create-base-query.ts | 13 +-- .../src/inject-mutation.ts | 8 +- .../src/providers.ts | 44 +++++----- 7 files changed, 31 insertions(+), 175 deletions(-) diff --git a/packages/angular-query-devtools-experimental/src/inject-devtools-panel.ts b/packages/angular-query-devtools-experimental/src/inject-devtools-panel.ts index 8035670851..7c558840cd 100644 --- a/packages/angular-query-devtools-experimental/src/inject-devtools-panel.ts +++ b/packages/angular-query-devtools-experimental/src/inject-devtools-panel.ts @@ -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 @@ -53,10 +55,7 @@ export function injectDevtoolsPanel( destroy, } - const destroyRef = inject(DestroyRef) - effect(() => { - const injectedClient = currentInjector.get(QueryClient, null) const { client = injectedClient, errorTypes = [], diff --git a/packages/angular-query-experimental/src/__tests__/inject-mutation-state.test.ts b/packages/angular-query-experimental/src/__tests__/inject-mutation-state.test.ts index a7e31b78ef..8379736ce0 100644 --- a/packages/angular-query-experimental/src/__tests__/inject-mutation-state.test.ts +++ b/packages/angular-query-experimental/src/__tests__/inject-mutation-state.test.ts @@ -143,7 +143,7 @@ describe('injectMutationState', () => { @Component({ selector: 'app-fake', template: ` - @for (mutation of mutationState(); track mutation) { + @for (mutation of mutationState(); track $index) { {{ mutation.status }} } `, diff --git a/packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts b/packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts index 35a3f58545..d774775c39 100644 --- a/packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts +++ b/packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts @@ -1,8 +1,6 @@ import { Component, - Injectable, Injector, - inject, input, provideExperimentalZonelessChangeDetection, signal, @@ -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((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(() => { diff --git a/packages/angular-query-experimental/src/__tests__/inject-query.test.ts b/packages/angular-query-experimental/src/__tests__/inject-query.test.ts index d0c526023f..39c660e172 100644 --- a/packages/angular-query-experimental/src/__tests__/inject-query.test.ts +++ b/packages/angular-query-experimental/src/__tests__/inject-query.test.ts @@ -1,10 +1,8 @@ import { Component, - Injectable, Injector, computed, effect, - inject, input, provideExperimentalZonelessChangeDetection, signal, @@ -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('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('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(() => { diff --git a/packages/angular-query-experimental/src/create-base-query.ts b/packages/angular-query-experimental/src/create-base-query.ts index d53a387087..5ef6ebbe28 100644 --- a/packages/angular-query-experimental/src/create-base-query.ts +++ b/packages/angular-query-experimental/src/create-base-query.ts @@ -1,12 +1,10 @@ import { DestroyRef, - Injector, NgZone, VERSION, computed, effect, inject, - runInInjectionContext, signal, untracked, } from '@angular/core' @@ -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 @@ -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 }) @@ -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, }, ) diff --git a/packages/angular-query-experimental/src/inject-mutation.ts b/packages/angular-query-experimental/src/inject-mutation.ts index cf6b86f384..09e933cad5 100644 --- a/packages/angular-query-experimental/src/inject-mutation.ts +++ b/packages/angular-query-experimental/src/inject-mutation.ts @@ -1,11 +1,9 @@ import { DestroyRef, - Injector, NgZone, computed, effect, inject, - runInInjectionContext, signal, untracked, } from '@angular/core' @@ -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' @@ -40,7 +39,6 @@ export function injectMutation< injector?: Injector, ): CreateMutationResult { return assertInjector(injectMutation, injector, () => { - const currentInjector = inject(Injector) const destroyRef = inject(DestroyRef) const ngZone = inject(NgZone) const queryClient = inject(QueryClient) @@ -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< diff --git a/packages/angular-query-experimental/src/providers.ts b/packages/angular-query-experimental/src/providers.ts index 42645b1b85..6c43fb417c 100644 --- a/packages/angular-query-experimental/src/providers.ts +++ b/packages/angular-query-experimental/src/providers.ts @@ -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' @@ -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: () => { @@ -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 @@ -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') @@ -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) + }) }) }, },