Enhance type safety in @ngrx/signals

[Chiorufarewerin]

I hadn't used @ngrx/signals before, so recently I decided to migrate some of my services to this store. It seems really promising, especially because of its declarative approach and extensibility.

However, I encountered an issue with type safety. I want to ensure that no store members can be directly modified, but currently, this is not the case.

Here is an example of a store:

export const UserStore = signalStore(
  { providedIn: 'root' },
  withEntities<User>(),
  withState<{ readonly requestStatus: string }>({ requestStatus: 'idle' }),
  withMethods((store, client = inject(UserClient)) => ({
    loadAll: rxMethod<void>(
      exhaustMap(() => {
        patchState(store, { requestStatus: 'pending' });

        return client.getUsers().pipe(
          tapResponse({
            next: (users) => {
              patchState(store, setAllEntities(users));
              patchState(store, { requestStatus: 'fulfilled' });
            },
            error: () => {
              patchState(store, removeAllEntities());
              patchState(store, { requestStatus: 'error' });
            },
          }),
        );
      }),
    ),
  })),
);

And here is an example of how the store can be modified:

export class MainComponent {
  readonly userStore = inject(UserStore);

  constructor() {
    this.userStore.loadAll();

    setTimeout(() => {
      this.userStore.entities = signal([{ id: 1000, name: 'Vova' }]); // Signal property can be modified ❌
      this.userStore.entityMap()[0] = { id: 2000, name: 'Dima' }; // entityMap can be modified directly ❌
      this.userStore.loadAll = rxMethod(tap(console.log)); // Method can be modified ❌
      this.userStore.requestStatus = signal('set') // Cannot assign ✅
    }, 3000);
  }
}

Suggestion

To improve type safety, I suggest making the signal store more restrictive by adding readonly to every type that should not allow direct modification.

[rainerhahnekamp]

@Chiorufarewerin, version 19.0.0 originally introduced built-in immutable protection, but we had to remove it because there are valid cases where developers require mutable changes. While these cases are rare, they do exist, so we ultimately had to revert the change. More details can be found here: #4683.

We also considered enforcing immutability using read-only types, but after consulting with the Angular team, we decided against it. TypeScript doesn’t always guarantee full type safety, which introduces certain challenges.

For those who still want immutable state enforcement, there is an opt-in community extension: withImmutableState in the ngrx-toolkit.

[rainerhahnekamp]

It’s a bit surprising that you're against using readonly

I am not against readonly per se, but to use inside of the update function of patchState

---

That said, I misread your suggestion 😅. You’re proposing to make the properties readonly, correct? That makes sense to me. I just hope there aren’t any technical obstacles we haven’t considered yet. As long as the state’s types remain unchanged, your approach should work.

[Chiorufarewerin]

Correct! Here is the example PR that reflects what I want. In fact, if you search through the code, there are more places where readonly could be applied, such as { state: {}; props: Props; methods: {} }. However, since these properties are only modified during the construction of the store using features, I decided to keep only what is essential.

[rainerhahnekamp]

We usually deal with changes to our codebase via issues. So I'm going to convert this one to an issue.

[rainerhahnekamp]

Btw. @Chiorufarewerin, if you want to create the issue yourself (stating you as author) be free to do so. I'd remove the converted issue.

[Chiorufarewerin]

That’s fine if I’m not the author. The important thing is that the problem is solved. 😺