From 5c6a9de7a01518ed0efebbeafb788c2da44fa7e5 Mon Sep 17 00:00:00 2001 From: Thilo Aschebrock Date: Sat, 3 May 2025 13:57:56 +1000 Subject: [PATCH 1/3] fix(angular-query): ensure initial mutation pending state is emitted --- .../src/__tests__/inject-mutation.test.ts | 2 - .../src/inject-mutation.ts | 70 ++++++++----------- 2 files changed, 31 insertions(+), 41 deletions(-) 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 3ea18743d7..af8f3a833d 100644 --- a/packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts +++ b/packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts @@ -58,8 +58,6 @@ describe('injectMutation', () => { })) }) - TestBed.tick() - mutation.mutate(result) vi.advanceTimersByTime(1) diff --git a/packages/angular-query-experimental/src/inject-mutation.ts b/packages/angular-query-experimental/src/inject-mutation.ts index 16b22ed2f7..2e125aa36e 100644 --- a/packages/angular-query-experimental/src/inject-mutation.ts +++ b/packages/angular-query-experimental/src/inject-mutation.ts @@ -84,14 +84,6 @@ export function injectMutation< } }) - /** - * Computed signal that gets result from mutation cache based on passed options - */ - const resultFromInitialOptionsSignal = computed(() => { - const observer = observerSignal() - return observer.getCurrentResult() - }) - /** * Signal that contains result set by subscriber */ @@ -102,44 +94,44 @@ export function injectMutation< TContext > | null>(null) - effect( - () => { - const observer = observerSignal() - const observerOptions = optionsSignal() + /** + * Computed signal that gets result from mutation cache based on passed options + */ + const resultFromInitialOptionsSignal = computed(() => { + const observer = observerSignal() - untracked(() => { - observer.setOptions(observerOptions) - }) - }, - { - injector, - }, - ) + untracked(() => { + const unsubscribe = ngZone.runOutsideAngular(() => + // observer.trackResult is not used as this optimization is not needed for Angular + observer.subscribe( + notifyManager.batchCalls((state) => { + ngZone.run(() => { + if ( + state.isError && + shouldThrowError(observer.options.throwOnError, [state.error]) + ) { + ngZone.onError.emit(state.error) + throw state.error + } + + resultFromSubscriberSignal.set(state) + }) + }), + ), + ) + destroyRef.onDestroy(unsubscribe) + }) + + return observer.getCurrentResult() + }) effect( () => { - // observer.trackResult is not used as this optimization is not needed for Angular const observer = observerSignal() + const observerOptions = optionsSignal() untracked(() => { - const unsubscribe = ngZone.runOutsideAngular(() => - observer.subscribe( - notifyManager.batchCalls((state) => { - ngZone.run(() => { - if ( - state.isError && - shouldThrowError(observer.options.throwOnError, [state.error]) - ) { - ngZone.onError.emit(state.error) - throw state.error - } - - resultFromSubscriberSignal.set(state) - }) - }), - ), - ) - destroyRef.onDestroy(unsubscribe) + observer.setOptions(observerOptions) }) }, { From 9fcd7dd5d5bfe45920b15997464cc0da39632b33 Mon Sep 17 00:00:00 2001 From: Thilo Aschebrock Date: Sat, 24 May 2025 10:21:35 +1000 Subject: [PATCH 2/3] Remove effects from injectMutation to avoid synchronisation issues --- .../src/__tests__/inject-mutation.test.ts | 12 +++++++-- .../src/inject-mutation.ts | 25 +++++++------------ 2 files changed, 19 insertions(+), 18 deletions(-) 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 af8f3a833d..d3ca629274 100644 --- a/packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts +++ b/packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts @@ -131,9 +131,17 @@ describe('injectMutation', () => { mutation.mutate('xyz') - const mutations = mutationCache.find({ mutationKey: ['2'] }) + mutationKey.set(['3']) - expect(mutations?.options.mutationKey).toEqual(['2']) + mutation.mutate('xyz') + + expect(mutationCache.find({ mutationKey: ['1'] })).toBeUndefined() + expect( + mutationCache.find({ mutationKey: ['2'] })?.options.mutationKey, + ).toEqual(['2']) + expect( + mutationCache.find({ mutationKey: ['3'] })?.options.mutationKey, + ).toEqual(['3']) }) test('should reset state after invoking mutation.reset', async () => { diff --git a/packages/angular-query-experimental/src/inject-mutation.ts b/packages/angular-query-experimental/src/inject-mutation.ts index 2e125aa36e..ad8fadc015 100644 --- a/packages/angular-query-experimental/src/inject-mutation.ts +++ b/packages/angular-query-experimental/src/inject-mutation.ts @@ -4,7 +4,6 @@ import { NgZone, assertInInjectionContext, computed, - effect, inject, signal, untracked, @@ -71,7 +70,15 @@ export function injectMutation< null return computed(() => { - return (instance ||= new MutationObserver(queryClient, optionsSignal())) + const observerOptions = optionsSignal() + return untracked(() => { + if (instance) { + instance.setOptions(observerOptions) + } else { + instance = new MutationObserver(queryClient, observerOptions) + } + return instance + }) }) })() @@ -125,20 +132,6 @@ export function injectMutation< return observer.getCurrentResult() }) - effect( - () => { - const observer = observerSignal() - const observerOptions = optionsSignal() - - untracked(() => { - observer.setOptions(observerOptions) - }) - }, - { - injector, - }, - ) - const resultSignal = computed(() => { const resultFromSubscriber = resultFromSubscriberSignal() const resultFromInitialOptions = resultFromInitialOptionsSignal() From 527574a4233fd21761c50833dd8bc7bebf3c3e57 Mon Sep 17 00:00:00 2001 From: Thilo Aschebrock Date: Sun, 25 May 2025 10:46:28 +1000 Subject: [PATCH 3/3] Add explicit test case for mutating in constructor --- .../__tests__/inject-mutation-state.test.ts | 1 - .../src/__tests__/inject-mutation.test.ts | 35 ++++++++++++++++--- .../src/__tests__/inject-query.test.ts | 1 - 3 files changed, 31 insertions(+), 6 deletions(-) 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 bf3c37f39b..9e72b5e245 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 @@ -144,7 +144,6 @@ describe('injectMutationState', () => { {{ mutation.status }} } `, - standalone: true, }) class FakeComponent { name = input.required() 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 d3ca629274..2ecccf882e 100644 --- a/packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts +++ b/packages/angular-query-experimental/src/__tests__/inject-mutation.test.ts @@ -316,7 +316,6 @@ describe('injectMutation', () => { {{ mutation.data() }} `, - standalone: true, }) class FakeComponent { name = input.required() @@ -339,8 +338,6 @@ describe('injectMutation', () => { button.triggerEventHandler('click') await resolveMutations() - fixture.detectChanges() - const text = debugElement.query(By.css('span')).nativeElement.textContent expect(text).toEqual('value') const mutation = mutationCache.find({ mutationKey: ['fake', 'value'] }) @@ -357,7 +354,6 @@ describe('injectMutation', () => { {{ mutation.data() }} `, - standalone: true, }) class FakeComponent { name = input.required() @@ -400,6 +396,37 @@ describe('injectMutation', () => { expect(mutation2!.options.mutationKey).toEqual(['fake', 'updatedValue']) }) + test('should have pending state when mutating in constructor', async () => { + @Component({ + selector: 'app-fake', + template: ` + {{ mutation.isPending() ? 'pending' : 'not pending' }} + `, + }) + class FakeComponent { + mutation = injectMutation(() => ({ + mutationKey: ['fake'], + mutationFn: () => sleep(0).then(() => 'fake'), + })) + + constructor() { + this.mutation.mutate() + } + } + + const fixture = TestBed.createComponent(FakeComponent) + const { debugElement } = fixture + const span = debugElement.query(By.css('span')) + + vi.advanceTimersByTime(1) + + expect(span.nativeElement.textContent).toEqual('pending') + + await resolveMutations() + + expect(span.nativeElement.textContent).toEqual('not pending') + }) + describe('throwOnError', () => { test('should evaluate throwOnError when mutation is expected to throw', async () => { const err = new Error('Expected mock error. All is well!') 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 44964b7927..0309296430 100644 --- a/packages/angular-query-experimental/src/__tests__/inject-query.test.ts +++ b/packages/angular-query-experimental/src/__tests__/inject-query.test.ts @@ -520,7 +520,6 @@ describe('injectQuery', () => { @Component({ selector: 'app-fake', template: `{{ query.data() }}`, - standalone: true, }) class FakeComponent { name = input.required()