Whether Signal Store withComputed should contain MethodsDictionary in store provided to it's factory

[k3nsei]

Currently, Signal Store does not allow methods to be used inside withComputed to prevent developers from "footgunning" themselves. But there are valid real world use cases where methods need to be used there. For example, to connect to another store or external resource and not having it as part of state (to not be wrapped into Signal or DeepSignal).

I strongly believe that libraries shouldn't do the thinking for developers and save them from stupid mistakes. For that there is eslint and eslint rule could be used to warn them about those potential "footguns". But the library shouldn't restrict its use.

[jits]

UPDATE: ahh, I see from #4499 that methods aren't exposed in withComputed(…), so ignore my suggestion, below.

---

You should be able to achieve this by defining multiple factory functions — each subsequent factory function has access to the previous declarations. Roughly speaking:

const Store = signalStore(
  withState(…),
  withComputed(…),
  withMethods(…),
  withComputed(…),
  // etc.
)

[k3nsei]

@jits I'm referring to quite advanced usages like abstract example below

import { computed, inject, type Signal, type Type } from "@angular/core";
import {
  patchState,
  signalStore,
  type SignalStoreFeature,
  type SignalStoreFeatureResult,
  withComputed,
  withMethods,
  withState,
} from "@ngrx/signals";
import { lowerFirst } from "lodash-es";

/* eslint-disable @typescript-eslint/ban-types */

type Connected<T> = (() => T) & T;

type UnwrapSignal<T> = T extends Signal<infer U> ? U : never;

const connect = <T extends object, U extends Connected<T>>(value: T): U =>
  new Proxy(() => value, {
    get: (_, prop) => Reflect.get(value, prop),
    has: (_, prop) => Reflect.has(value, prop),
  }) as U;

export const withConnectedStore = <
  ConnectedStore extends Type<unknown>,
  Name extends string,
  Input extends SignalStoreFeatureResult = SignalStoreFeatureResult,
>(
  name: Name,
  connectedStoreRef: ConnectedStore
): SignalStoreFeature<
  Input,
  { state: {}; computed: {}; methods: Record<typeof prop, Connected<NoInfer<InstanceType<ConnectedStore>>>> }
> => {
  const prop = `${lowerFirst(name)}Store` as `${Uncapitalize<Name>}Store`;

  return (store) => {
    const connectedStore = inject(connectedStoreRef);

    return {
      ...store,
      methods: {
        ...store.methods,
        [prop]: connect(connectedStore as object),
      } as Record<typeof prop, Connected<NoInfer<InstanceType<ConnectedStore>>>>,
    };
  };
};

const OrderStore = signalStore(
  withState<{
    by: string | null;
    direction: "ASC" | "DESC";
  }>({
    by: null,
    direction: "ASC",
  }),
  withMethods((store) => ({
    change(by: string | null, direction: "ASC" | "DESC"): void {
      return patchState(store, { by, direction });
    },
    applyParams(params: URLSearchParams): URLSearchParams {
      const orderBy = store.by();
      const orderDirection = store.direction();

      if (orderBy) {
        params.append("orderBy", `${orderBy}:${orderDirection}`);
      }

      return params;
    },
  }))
);

const FiltersStore = signalStore(
  withState<{
    name: string | null;
    status: "created" | "approved" | "active" | null;
  }>({
    name: null,
    status: null,
  }),
  withMethods((store) => ({
    setFilter<T extends "name" | "status">(prop: T, value: UnwrapSignal<(typeof store)[T]>): void {
      return patchState(store, { [prop]: value });
    },
    applyParams(params: URLSearchParams): URLSearchParams {
      const nameFilter = store.name();
      const statusFilter = store.status();

      if (nameFilter) {
        params.append("name", nameFilter);
      }

      if (statusFilter) {
        params.append("status", statusFilter);
      }

      return params;
    },
  }))
);

const DataSourceStore = signalStore(
  withState({
    isLoading: false,
    isLoaded: false,
    data: [],
  }),
  withConnectedStore("order", OrderStore),
  withConnectedStore("filters", FiltersStore),
  withComputed((store) => ({
    query: computed<string>(() => {
      const params = new URLSearchParams();

      // I know that those stores could be injected here using `inject()`, it serves just as an example.
      // In my real application I'm connecting here Tanstack Angular Query
      store.orderStore.applyParams(params);
      store.filtersStore.applyParams(params);

      return params.toString();
    }),
  }))
  // withMethods(() => ({
  // 	methods required to fetch and store data etc.
  // }))
);

[rainerhahnekamp]

Here are my thoughts:

1. You can always access signals from other stores via Dependency Injection (DI) in withComputed. The injection should occur outside of the computed, similar to how withMethods operates.
2. I would question the need for a computed to run side-effects. This seems like a very rare edge case, and there are likely better ways to handle it. Instead of using computed, consider using an rxMethod that sets a property in the state. From an external perspective, the result would be the same.
3. Many developers select a library because it provides clear guidance on best practices. This applies to ngrx as well. Consider the constraints in ngrx, such as no async operations or DI in reducers within the global store, or the Angular framework’s limitations. These restrictions exist for good reasons.

[markostanimirovic]

Can you share a code example of how you're trying to integrate Angular Query with SignalStore? It doesn't have to be the code snippet from your company's codebase, but the general one.

In the provided example, side effects are performed within the computed. Unrelated to Angular Signals and SignalStore, this is not recommended - derived state computation should not have any side effects. As Rainer mentioned, there are cleaner ways to achieve the same behavior by keeping derived state computations pure.

[k3nsei]

@markostanimirovic I don't know about what side-effects you are talking about? In provided example what is consumed is pure function which using signals from other stores using common interface to split code by responsibilities (filers, sorting and data fetching). Signals are right things to consume there and it's my responsibility as a developer to know what I'm doing not signal store.

[k3nsei]

Here is example: https://stackblitz.com/edit/stackblitz-starters-rb4fxr
Here is example with applied patch: https://stackblitz.com/edit/stackblitz-starters-j5e1h6

import { computed, DestroyRef, inject } from '@angular/core';
import { takeUntilDestroyed } from '@angular/core/rxjs-interop';

import {
  patchState,
  signalStore,
  withComputed,
  withMethods,
  withState,
} from '@ngrx/signals';
import { withQuery } from '@ngx-signal-store-query/core';
import { lastValueFrom, of } from 'rxjs';
import { delay } from 'rxjs/operators';

interface ExampleState {
  selectedCategoryId: string | null;
}

export interface Category {
  id: string;
  name: string;
  subCategoryIds: string[];
}

export type SubCategory = Omit<Category, 'subCategoryIds'>;

export const ExampleStore = signalStore(
  withState<ExampleState>({ selectedCategoryId: null }),
  withMethods((store) => ({
    select(category: Category | null | undefined): void {
      return patchState(store, (state) => ({
        selectedCategoryId:
          category != null && !Object.is(category.id, state.selectedCategoryId)
            ? category.id
            : null,
      }));
    },
  })),
  withQuery('categories', () => {
    const destroyRef = inject(DestroyRef);

    return () => ({
      queryKey: ['api', 'v1', 'categories'],
      queryFn(): Promise<Category[]> {
        return lastValueFrom(
          of([
            {
              id: '814ff3dc-8e9c-44ce-ac93-d4f9c537ad92',
              name: 'Cat. A',
              subCategoryIds: [
                '6307dcf8-8ce6-4fe7-9f54-21d2b24614e0',
                'cad8c499-36e9-4ee5-8e7d-0f44647577b2',
              ],
            },
            {
              id: '176003a9-b273-4f8b-aa6a-16ecf1af07e7',
              name: 'Cat. B',
              subCategoryIds: [
                '27c15899-9492-4c36-925f-6d6347d63299',
                'cea6dd52-db59-4087-8258-a9120b4a7542',
              ],
            },
          ]).pipe(delay(1000), takeUntilDestroyed(destroyRef))
        );
      },
    });
  }),
  withQuery('subCategories', (store) => {
    const destroyRef = inject(DestroyRef);

    return () => {
      const categoryId = store.selectedCategoryId();

      return {
        enabled: categoryId != null,
        queryKey: [
          'api',
          'v1',
          'category',
          { categoryId: categoryId ?? null },
          'subCategories',
        ],
        queryFn(): Promise<SubCategory[]> {
          return lastValueFrom(
            of([
              {
                id: '6307dcf8-8ce6-4fe7-9f54-21d2b24614e0',
                name: 'SubCat. A',
              },
              {
                id: 'cad8c499-36e9-4ee5-8e7d-0f44647577b2',
                name: 'SubCat. B',
              },
            ]).pipe(delay(500), takeUntilDestroyed(destroyRef))
          );
        },
      };
    };
  }),
  withComputed((store) => ({
    // Here is a problem that `categoriesQuery` and `subCategoriesQuery` are not accessible because they come from methods
    isLoadingCategories: store.categoriesQuery.isFetching,
    isLoadingSubCategories: store.subCategoriesQuery.isFetching,
    categories: computed<Category[]>(() => store.categoriesQuery.data() ?? []),
    subCategories: computed<SubCategory[]>(
      () => store.subCategoriesQuery.data() ?? []
    ),
  }))
);

[markostanimirovic]

A new RFC for adding the withProps base SignalStore feature is opened: #4504

In addition to other use cases, it would allow defining aliases:

const BooksStore = signalStore(
  withQuery('books', /* ... */),
  withProps(({ booksQuery }) => ({
    books: booksQuery.data,
    isFetching: booksQuery.isFetching,
  }))
);

[k3nsei]

@markostanimirovic that's exactly what I was talking about with @rainerhahnekamp last week on discord that it's something like withProps or withExplicit is needed as Signal Store primitive.

[k3nsei]

Closing as RFC #4503 would bring requested capabilities