refactor(devtools): devtools destruction on renamed name

Author: rainerhahnekamp

apps/demo/src/app/devtools/todo-detail.component.ts

@@ -2,7 +2,10 @@ import { Component, effect, inject, input } from '@angular/core';
 import { MatCard, MatCardModule } from '@angular/material/card';
 import { Todo } from './todo-store';
 import { signalStore, withState } from '@ngrx/signals';
-import { withDevtools } from '@angular-architects/ngrx-toolkit';
+import {
+  renameDevtoolsName,
+  withDevtools,
+} from '@angular-architects/ngrx-toolkit';
 
 /**
  * This Store can be instantiated multiple times, if the user
@@ -38,4 +41,13 @@ const TodoDetailStore = signalStore(
 export class TodoDetailComponent {
   readonly #todoDetailStore = inject(TodoDetailStore);
   todo = input.required<Todo>();
+
+  constructor() {
+    effect(
+      () => {
+        renameDevtoolsName(this.#todoDetailStore, `todo-${this.todo().id}`);
+      },
+      { allowSignalWrites: true }
+    );
+  }
 }

apps/demo/src/app/devtools/todo-store.ts

@@ -50,9 +50,18 @@ export const TodoStore = signalStore(
           updateEntity({ id, changes: { finished: !todo.finished } })
         );
       },
-      selectTodo(id: number) {
-        updateState(store, `select todo ${id}`, {
-          selectedIds: [...store.selectedIds(), id],
+      toggleSelectTodo(id: number) {
+        updateState(store, `select todo ${id}`, ({ selectedIds }) => {
+          if (selectedIds.includes(id)) {
+            return {
+              selectedIds: selectedIds.filter(
+                (selectedId) => selectedId !== id
+              ),
+            };
+          }
+          return {
+            selectedIds: [...store.selectedIds(), id],
+          };
         });
       },
     };

apps/demo/src/app/devtools/todo.component.ts

@@ -86,7 +86,7 @@ export class TodoComponent {
   }
 
   checkboxLabel(todo: Todo) {
-    this.todoStore.selectTodo(todo.id);
+    this.todoStore.toggleSelectTodo(todo.id);
   }
 
   removeTodo(todo: Todo) {

libs/ngrx-toolkit/src/lib/devtools/internal/devtools-syncer.service.ts

@@ -30,6 +30,7 @@ const dummyConnection: Connection = {
 export class DevtoolsSyncer implements OnDestroy {
   readonly #stores = signal<StoreRegistry>({});
   readonly #isBrowser = isPlatformBrowser(inject(PLATFORM_ID));
+  #currentId = 1;
 
   readonly #connection: Connection = this.#isBrowser
     ? window.__REDUX_DEVTOOLS_EXTENSION__
@@ -39,9 +40,6 @@ export class DevtoolsSyncer implements OnDestroy {
       : dummyConnection
     : dummyConnection;
 
-  // keeps track of names which have already been synced. Synced names cannot be renamed
-  readonly #syncedStoreNames = new Set();
-
   constructor() {
     if (!this.#isBrowser) {
       return;
@@ -62,7 +60,6 @@ export class DevtoolsSyncer implements OnDestroy {
       const stores = this.#stores();
       const rootState: Record<string, unknown> = {};
       for (const name in stores) {
-        this.#syncedStoreNames.add(name);
         const { store } = stores[name];
         rootState[name] = getState(store);
       }
@@ -94,41 +91,38 @@ Enable automatic indexing via withDevTools('${storeName}', { indexNames: true })
     for (let i = 1; names.includes(storeName); i++) {
       storeName = `${name}-${i}`;
     }
+    const id = this.#currentId++;
 
     this.#stores.update((stores) => ({
       ...stores,
-      [storeName]: { store, options },
+      [storeName]: { store, options, id },
     }));
+
+    return id;
   }
 
-  removeStore(name: string) {
-    this.#stores.update((value) => {
-      const newStore: StoreRegistry = {};
-      for (const storeName in value) {
-        if (storeName !== name) {
-          newStore[storeName] = value[storeName];
+  removeStore(id: number) {
+    this.#stores.update((stores) => {
+      return Object.entries(stores).reduce((newStore, [name, value]) => {
+        if (value.id === id) {
+          return newStore;
+        } else {
+          return { ...newStore, [name]: value };
         }
-      }
-
-      return newStore;
+      }, {});
     });
   }
 
   renameStore(oldName: string, newName: string) {
-    if (this.#syncedStoreNames.has(oldName)) {
-      throw new Error(
-        `NgRx Toolkit/DevTools: cannot rename from ${oldName} to ${newName}. ${oldName} has already been send to DevTools.`
-      );
-    }
-
     this.#stores.update((stores) => {
+      if (newName in stores) {
+        throw new Error(
+          `NgRx Toolkit/DevTools: cannot rename from ${oldName} to ${newName}. ${oldName} does not exist.`
+        );
+      }
+
       const newStore: StoreRegistry = {};
       for (const storeName in stores) {
-        if (storeName === newName) {
-          throw new Error(
-            `NgRx Toolkit/DevTools: cannot rename from ${oldName} to ${newName}. ${newName} already exists.`
-          );
-        }
         if (storeName === oldName) {
           newStore[newName] = stores[oldName];
         } else {
@@ -143,5 +137,5 @@ Enable automatic indexing via withDevTools('${storeName}', { indexNames: true })
 
 type StoreRegistry = Record<
   string,
-  { store: StateSource<object>; options: DevtoolsOptions }
+  { store: StateSource<object>; options: DevtoolsOptions; id: number }
 >;

libs/ngrx-toolkit/src/lib/devtools/rename-devtools-name.ts

@@ -3,9 +3,6 @@ import { renameDevtoolsMethodName } from './with-devtools';
 
 /**
  * Renames the name of a store how it appears in the Devtools.
- *
- * This method has to be executed before the first
- * synchronization. In most cases. that's in the constructor.
  * @param store instance of the SignalStore
  * @param newName new name for the Devtools
  */

libs/ngrx-toolkit/src/lib/devtools/tests/basic.spec.ts

@@ -2,9 +2,16 @@ import { setupExtensions } from './helpers';
 import { TestBed, waitForAsync } from '@angular/core/testing';
 import { patchState, signalStore, withMethods, withState } from '@ngrx/signals';
 import { withDevtools } from '../with-devtools';
-import { Component, inject } from '@angular/core';
+import {
+  Component,
+  createEnvironmentInjector,
+  EnvironmentInjector,
+  inject,
+  Injector,
+} from '@angular/core';
 import { provideRouter } from '@angular/router';
 import { RouterTestingHarness } from '@angular/router/testing';
+import { renameDevtoolsName } from '../rename-devtools-name';
 
 describe('Devtools Basics', () => {
   it('should dispatch update', () => {
@@ -38,44 +45,52 @@ describe('Devtools Basics', () => {
     );
   });
 
-  it('should remove the state once destroyed', waitForAsync(async () => {
+  it('should remove the state once destroyed', () => {
     const { sendSpy } = setupExtensions();
 
     const Store = signalStore(withDevtools('flight'));
+    const childInjector = createEnvironmentInjector(
+      [Store],
+      TestBed.inject(EnvironmentInjector)
+    );
 
-    @Component({
-      selector: 'app-flight-search',
-      template: ``,
-      standalone: true,
-      providers: [Store],
-    })
-    class FlightSearchComponent {
-      store = inject(Store);
-    }
+    childInjector.get(Store);
+    TestBed.flushEffects();
 
-    @Component({ selector: 'app-home', template: ``, standalone: true })
-    class HomeComponent {}
+    expect(sendSpy).toHaveBeenCalledWith(
+      { type: 'Store Update' },
+      { flight: {} }
+    );
 
-    TestBed.configureTestingModule({
-      providers: [
-        provideRouter([
-          { path: '', component: HomeComponent },
-          { path: 'flight', component: FlightSearchComponent },
-        ]),
-      ],
-    });
+    childInjector.destroy();
+    TestBed.flushEffects();
+    expect(sendSpy).toHaveBeenCalledWith({ type: 'Store Update' }, {});
+  });
+
+  it('should remove a renamed state once destroyed', () => {
+    const { sendSpy } = setupExtensions();
+
+    const Store = signalStore(withDevtools('flight'));
+    const childInjector = createEnvironmentInjector(
+      [Store],
+      TestBed.inject(EnvironmentInjector)
+    );
+
+    const store = childInjector.get(Store);
+    TestBed.flushEffects();
 
-    const harness = await RouterTestingHarness.create('flight');
     expect(sendSpy).toHaveBeenCalledWith(
       { type: 'Store Update' },
       { flight: {} }
     );
-    await harness.navigateByUrl('/');
+
+    renameDevtoolsName(store, 'flights');
+    childInjector.destroy();
     TestBed.flushEffects();
     expect(sendSpy).toHaveBeenCalledWith({ type: 'Store Update' }, {});
-  }));
+  });
 
-  it('should group multiple patchState running before the first synchronization', () => {
+  it('should group multiple patchState running before the synchronization', () => {
     const { sendSpy } = setupExtensions();
     const store = TestBed.inject(
       signalStore(

libs/ngrx-toolkit/src/lib/devtools/tests/naming.spec.ts

@@ -130,19 +130,22 @@ Enable automatic indexing via withDevTools('flights', { indexNames: true }), or
       );
     });
 
-    it('should throw on rename after sync', () => {
-      setupExtensions();
+    it('should also rename after sync', () => {
+      const { sendSpy } = setupExtensions();
       const Store = signalStore(
         { providedIn: 'root' },
         withState({ name: 'Product', price: 10.5 }),
         withDevtools('flight')
       );
       const store = TestBed.inject(Store);
 
+      TestBed.flushEffects();
+      renameDevtoolsName(store, 'flights');
       TestBed.flushEffects();
 
-      expect(() => renameDevtoolsName(store, 'flights')).toThrow(
-        'NgRx Toolkit/DevTools: cannot rename from flight to flights. flight has already been send to DevTools.'
+      expect(sendSpy).toHaveBeenCalledWith(
+        { type: 'Store Update' },
+        { flights: { name: 'Product', price: 10.5 } }
       );
     });
 

libs/ngrx-toolkit/src/lib/devtools/with-devtools.ts

@@ -1,4 +1,10 @@
-import { signalStoreFeature, withHooks, withMethods } from '@ngrx/signals';
+import {
+  SignalStoreFeature,
+  signalStoreFeature,
+  SignalStoreFeatureResult,
+  withHooks,
+  withMethods,
+} from '@ngrx/signals';
 import { inject } from '@angular/core';
 import { DevtoolsSyncer } from './internal/devtools-syncer.service';
 
@@ -53,6 +59,7 @@ export type DevtoolsOptions = {
 export const existingNames = new Map<string, unknown>();
 
 export const renameDevtoolsMethodName = '___renameDevtoolsName';
+export const uniqueDevtoolsId = '___uniqueDevtoolsId';
 
 /**
  * Adds this store as a feature state to the Redux DevTools.
@@ -62,7 +69,7 @@ export const renameDevtoolsMethodName = '___renameDevtoolsName';
  * parameter the action name.
  *
  * The standalone function {@link renameDevtoolsName} can rename
- * the store name before the first synchronization.
+ * the store name.
  *
  * @param name name of the store as it should appear in the DevTools
  * @param options options for the DevTools
@@ -81,17 +88,20 @@ export function withDevtools(
   return signalStoreFeature(
     withMethods((store) => {
       const syncer = inject(DevtoolsSyncer);
-      syncer.addStore(name, store, finalOptions);
+      const id = syncer.addStore(name, store, finalOptions);
 
+      // TODO: use withProps and symbols
       return {
         [renameDevtoolsMethodName]: (newName: string) => {
           syncer.renameStore(name, newName);
         },
-      } as Record<string, (newName: string) => void>;
+        [uniqueDevtoolsId]: () => id,
+      } as Record<string, (newName?: unknown) => unknown>;
     }),
-    withHooks(() => {
+    withHooks((store) => {
       const syncer = inject(DevtoolsSyncer);
-      return { onDestroy: () => syncer.removeStore(name) };
+      const id = Number(store[uniqueDevtoolsId]());
+      return { onDestroy: () => syncer.removeStore(id) };
     })
   );
 }