refactor: change mutation to object

Author: manfredsteyer

libs/ngrx-toolkit/src/lib/mutation.ts

@@ -1,8 +1,9 @@
-import { Injector } from '@angular/core';
+import { Injector, signal } from '@angular/core';
 import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
 import {
   catchError,
   concatMap,
+  finalize,
   Observable,
   ObservableInput,
   ObservedValueOf,
@@ -11,7 +12,8 @@ import {
   Subject,
   tap,
 } from 'rxjs';
-import { MutationResult } from './with-mutations';
+
+import { Mutation, MutationResult, MutationStatus } from './with-mutations';
 
 export type Func<P, R> = (params: P) => R;
 
@@ -29,43 +31,70 @@ export interface RxMutationOptions<P, R> {
 
 export function rxMutation<P, R>(
   options: RxMutationOptions<P, R>,
-): Func<P, Promise<MutationResult>> {
-  const mutationSubject = new Subject<P>();
+): Mutation<P> {
+  const inputSubject = new Subject<{
+    param: P;
+    resolve: (result: MutationResult) => void;
+  }>();
   const flatten = options.operator ?? concatMap;
 
   // TODO: Use injector
 
-  mutationSubject
+  const status = signal<MutationStatus>('idle');
+  const callCount = signal(0);
+  const errorSignal = signal<unknown>(undefined);
+
+  inputSubject
     .pipe(
-      flatten((param: P) =>
-        options.operation(param).pipe(
+      flatten((input) =>
+        options.operation(input.param).pipe(
           tap((result: R) => {
-            options.onSuccess?.(result, param);
-            // TODO: Decrease counter
+            options.onSuccess?.(result, input.param);
+            status.set('success');
+            input.resolve({
+              status: 'success',
+            });
           }),
           catchError((error: unknown) => {
-            const mutationError =
-              options.onError?.(error, param) ?? error ?? 'Mutation failed';
-            // TODO: Decrease counter
-            // TODO: Set mutationError
-            console.error('mutation error', mutationError);
+            options.onError?.(error, input.param);
+            const mutationError = error ?? 'Mutation failed';
+            errorSignal.set(mutationError);
+            status.set('error');
+            input.resolve({
+              status: 'error',
+              error: mutationError,
+            });
             return of(null);
           }),
+          finalize(() => {
+            callCount.update((c) => c - 1);
+            if (status() === 'processing') {
+              input.resolve({
+                status: 'aborted',
+              });
+            }
+          }),
         ),
       ),
       takeUntilDestroyed(),
     )
     .subscribe();
 
-  return (param: P) => {
+  const mutationFn = (param: P) => {
     return new Promise<MutationResult>((resolve) => {
-      // TODO: Increase Counter
-      mutationSubject.next(param);
-
-      // TODO: resolve promise when done
-      resolve({
-        status: 'success',
+      callCount.update((c) => c + 1);
+      status.set('processing');
+      inputSubject.next({
+        param,
+        resolve,
       });
     });
   };
+
+  const mutation = mutationFn as Mutation<P>;
+  mutation.status = status;
+  mutation.callCount = callCount;
+  mutation.error = errorSignal;
+
+  return mutation;
 }

libs/ngrx-toolkit/src/lib/rx-mutation.ts

@@ -1,13 +1,25 @@
 import { fakeAsync, TestBed, tick } from '@angular/core/testing';
 import { patchState, signalStore, withState } from '@ngrx/signals';
-import { delay, Observable, of, switchMap } from 'rxjs';
+import { delay, Observable, of, switchMap, throwError } from 'rxjs';
 import { rxMutation } from './rx-mutation';
 import { withMutations } from './with-mutations';
 
+async function asyncTick(): Promise<void> {
+  return new Promise((resolve) => {
+    setTimeout(() => {
+      resolve();
+    }, 1000);
+  });
+}
+
 function calcDouble(value: number): Observable<number> {
   return of(value * 2).pipe(delay(1000));
 }
 
+function fail(_value: number): Observable<number> {
+  return throwError(() => ({ error: 'Test-Error' })).pipe(delay(1000));
+}
+
 describe('mutation', () => {
   it('rxMutation should update the state', fakeAsync(() => {
     TestBed.runInInjectionContext(() => {
@@ -27,12 +39,55 @@ describe('mutation', () => {
         })),
       );
       const store = new Store();
+
+      expect(store.incrementStatus()).toEqual('idle');
+      expect(store.incrementProcessing()).toEqual(false);
+
       store.increment(2);
+      expect(store.incrementStatus()).toEqual('processing');
+      expect(store.incrementProcessing()).toEqual(true);
+
       tick(2000);
+      expect(store.incrementStatus()).toEqual('success');
+      expect(store.incrementProcessing()).toEqual(false);
+      expect(store.incrementError()).toEqual(undefined);
+
       expect(store.counter()).toEqual(7);
     });
   }));
 
+  it('rxMutation sets error', fakeAsync(() => {
+    TestBed.runInInjectionContext(() => {
+      const Store = signalStore(
+        withState({ counter: 3 }),
+        withMutations((store) => ({
+          increment: rxMutation({
+            operation: (value: number) => {
+              return fail(value);
+            },
+            onSuccess: (result) => {
+              patchState(store, (state) => ({
+                counter: state.counter + result,
+              }));
+            },
+          }),
+        })),
+      );
+      const store = new Store();
+
+      store.increment(2);
+
+      tick(2000);
+      expect(store.incrementStatus()).toEqual('error');
+      expect(store.incrementProcessing()).toEqual(false);
+      expect(store.incrementError()).toEqual({
+        error: 'Test-Error',
+      });
+
+      expect(store.counter()).toEqual(3);
+    });
+  }));
+
   it('rxMutation deals with race conditions', fakeAsync(() => {
     let onSuccessCalls = 0;
     let onErrorCalls = 0;
@@ -68,10 +123,18 @@ describe('mutation', () => {
       const store = new Store();
 
       store.increment(1);
+
       tick(500);
+      expect(store.incrementStatus()).toEqual('processing');
+      expect(store.incrementProcessing()).toEqual(true);
+
       store.increment(2);
       tick(1000);
 
+      expect(store.incrementStatus()).toEqual('success');
+      expect(store.incrementProcessing()).toEqual(false);
+      expect(store.incrementError()).toEqual(undefined);
+
       expect(store.counter()).toEqual(7);
       expect(onSuccessCalls).toEqual(1);
       expect(onErrorCalls).toEqual(0);
@@ -81,4 +144,38 @@ describe('mutation', () => {
       });
     });
   }));
+
+  it('rxMutation informs about aborted operations', async () => {
+    await TestBed.runInInjectionContext(async () => {
+      const Store = signalStore(
+        withState({ counter: 3 }),
+        withMutations((store) => ({
+          increment: rxMutation({
+            operation: (value: number) => {
+              return calcDouble(value);
+            },
+            operator: switchMap,
+            onSuccess: (result, params) => {
+              patchState(store, (state) => ({
+                counter: state.counter + result,
+              }));
+            },
+          }),
+        })),
+      );
+
+      const store = new Store();
+
+      const p1 = store.increment(1);
+      const p2 = store.increment(2);
+
+      await asyncTick();
+
+      const result1 = await p1;
+      const result2 = await p2;
+
+      expect(result1.status).toEqual('aborted');
+      expect(result2.status).toEqual('success');
+    });
+  });
 });

libs/ngrx-toolkit/src/lib/with-mutations.spec.ts

@@ -1,24 +1,29 @@
 import { computed, Signal } from '@angular/core';
 import {
+  EmptyFeatureResult,
   signalStoreFeature,
   SignalStoreFeature,
   SignalStoreFeatureResult,
   StateSignals,
   StateSource,
   withComputed,
   withMethods,
-  withState,
   WritableStateSource,
 } from '@ngrx/signals';
 
-// NamedMutationMethods below will infer the actual parameter and return types
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-type MutationsDictionary = Record<string, Mutation<any, any>>;
+export type Mutation<P> = {
+  (params: P): Promise<MutationResult>;
+  status: Signal<MutationStatus>;
+  callCount: Signal<number>;
+  error: Signal<unknown>;
+};
 
-export type Mutation<P, R> = (params: P) => Promise<R>;
+// NamedMutationMethods below will infer the actual parameter and return types
+type MutationsDictionary = Record<string, Mutation<never>>;
 
 export type MutationResult = {
-  status: 'success' | 'aborted';
+  status: 'success' | 'aborted' | 'error';
+  error?: unknown;
 };
 
 export type MutationStatus = 'idle' | 'processing' | 'error' | 'success';
@@ -27,32 +32,25 @@ export type MutationStatus = 'idle' | 'processing' | 'error' | 'success';
 // eslint-disable-next-line @typescript-eslint/no-unsafe-function-type
 export type MethodsDictionary = Record<string, Function>;
 
-type NamedMutationState<T extends MutationsDictionary> = {
-  [Prop in keyof T as `_${Prop & string}Count`]: number;
-} & {
-  [Prop in keyof T as `${Prop & string}Status`]: MutationStatus;
-} & {
-  [Prop in keyof T as `${Prop & string}Error`]: Error | undefined;
-};
-
 type NamedMutationProps<T extends MutationsDictionary> = {
   [Prop in keyof T as `${Prop & string}Processing`]: Signal<boolean>;
+} & {
+  [Prop in keyof T as `${Prop & string}Status`]: Signal<MutationStatus>;
+} & {
+  [Prop in keyof T as `${Prop & string}Error`]: Signal<Error | undefined>;
 };
 
 type NamedMutationMethods<T extends MutationsDictionary> = {
-  [Prop in keyof T as `${Prop & string}`]: T[Prop] extends Mutation<
-    infer P,
-    infer R
-  >
-    ? (p: P) => Promise<R>
+  [Prop in keyof T as `${Prop & string}`]: T[Prop] extends Mutation<infer P>
+    ? Mutation<P>
     : never;
 };
 
-export type NamedMutationResult<T extends MutationsDictionary> = {
-  state: NamedMutationState<T>;
-  props: NamedMutationProps<T>;
-  methods: NamedMutationMethods<T>;
-};
+export type NamedMutationResult<T extends MutationsDictionary> =
+  EmptyFeatureResult & {
+    props: NamedMutationProps<T>;
+    methods: NamedMutationMethods<T>;
+  };
 
 export function withMutations<
   Input extends SignalStoreFeatureResult,
@@ -94,39 +92,32 @@ function createMutationsFeature<Result extends MutationsDictionary>(
 ) {
   const keys = Object.keys(mutations);
 
-  const initState = keys.reduce(
-    (acc, key) => ({
-      ...acc,
-      [`_${key}Count`]: 0,
-      [`${key}Status`]: 'idle',
-      [`${key}Error`]: undefined,
-    }),
-    {} as NamedMutationState<Result>,
-  );
-
   const feature = signalStoreFeature(
-    withState(initState),
     withMethods(() =>
       keys.reduce(
         (acc, key) => ({
           ...acc,
-          [key]: async (params: unknown) => {
+          [key]: async (params: never) => {
             const mutation = mutations[key];
-            if (!mutation) throw new Error(`Mutation ${key} not found`);
+            if (!mutation) {
+              throw new Error(`Mutation ${key} not found`);
+            }
             const result = await mutation(params);
             return result;
           },
         }),
         {} as MethodsDictionary,
       ),
     ),
-    withComputed((store) =>
+    withComputed(() =>
       keys.reduce(
         (acc, key) => ({
           ...acc,
           [`${key}Processing`]: computed(() => {
-            return store[`_${key}Count`]() > 0;
+            return mutations[key].callCount() > 0;
           }),
+          [`${key}Status`]: mutations[key].status,
+          [`${key}Error`]: mutations[key].error,
         }),
         {} as NamedMutationProps<Result>,
       ),