RFC: Allow more than 10 signal store features about platform

[rainerhahnekamp]

I would like to bring in an alternative. We could apply a builder pattern. Its implementation would allow us to have as many features as we want.

It would look like this

const Store = signalStoreBuilder()
  .add(withState({ counter: 1 }))
  .add(
    withMethods((store) => {
      return {
        increment() {
          patchState(store, ({ counter }) => ({ counter: counter + 1 }));
        },
      };
    })
  )
  .add(
    withComputed((state) => {
      return {
        double: computed(() => state.counter() * 2),
      };
    })
  )
  .build();

signalStoreBuilder calls internally signalStore. There is already a proof of concept in https://github.com/rainerhahnekamp/ngrx/blob/feat/signals/builder/modules/signals/spec/signal-store-builder.spec.ts

Since the usage of withState/Computed/Methods is so common, there are also utility functions that makes it even easier:

const Store = signalStoreBuilder()
  .addState({ counter: 1 })
  .addMethods((store) => {
    return {
      increment() {
        patchState(store, (value) => ({ counter: value.counter + 1 }));
      },
    };
  })
  .addComputed((state) => ({
    double: computed(() => state.counter() * 2),
  }))
  .build();

Again, there is no limitation on how often add* can be called.

[gabrielguerrero]

@markostanimirovic , I think this is a good idea. I faced this and know people who had this problem too, it is easy to reach the max 9 params, and in signalStoreFeature is easy to reach the max 6 it has. As mentioned, it has a workaround of using signalStoreFeature to group it, but it would be nicer if it had more params; if you guys agree this should be implemented, I am happy to help create a PR.

[rainerhahnekamp]

The implementations of the Builder are, of course, up for discussion. It is not a breaking change. SincesignalStore is tree-shakable and the builder isn't (thanks to @mikezks), I'd say the signalStore should stay as the primary function to generate a signal store.

At the same time that doesn't mean that we shouldn't extend signalStore and signalStoreFeature.

[gabrielguerrero]

@rainerhahnekamp With builder pattern I don't know how to group features together and the builder pattern isn't tree-shakable.

Maybe some kind of Observable.pipe? signalStore already have signalStoreFeature, вut it does not have an type input through which it knows information about previous features. Is it passable to add it?

signalStore(
  withState()
  withComputed()
  signalStoreFeature(
    withMethods((store) => ...) // ⇽ cannot get value from `withComputed`
  )
)

signalStoreFeature has a first param where you can define the previous types it depends on

signalStoreFeature(type<{
    state: EntityState<Entity>;
    signals: EntitySignals<Entity>;
    methods: {};
  }>(), withMethods,....

but yeah it does not auto-get the previous type of the store in the withMethods use inside, I think I have an idea that could make that work, I will investigate

[gabrielguerrero]

@markostanimirovic, I did an investigation into making signalStoreFeature get the type of the store when it is inline, so it works similar to rxjs pipe operator. I managed to make it work in the fork branch at the end of the comment, is backwards compatible, all tests in the signals project pass (great tests by the way it really help me find all corner cases), I also added an extra one to test the inline case (I could add more if you guys want).
Example of use:

const Store = signalStore(
      withCustomFeature1(),
      withComputed(({ bar }) => ({
        s: computed(() => bar() + 's'),
      })),
      signalStoreFeature(
        withState({ foo2: 'foo2' }),
        withState({ bar2: 'bar2' }),
        withComputed(({ bar2 }) => ({
          s2: computed(() => bar2() + 's'),
        })),
        withMethods(({ foo, bar, baz, s, foo2, bar2, s2 }) => ({
          all: () => foo() + bar() + baz() + s() + foo2() + bar2() + s2(),
        }))
      )
    );

Notice that the last withMethods can access everything defined before it, regardless of whether it is part of the parent store or another previous store feature. The only thing is that it requires Ts 5.4 NoInfer, so we might need to wait until the Angular 18 release if you guys think this should be added.

Below is the fork, I did not create a PR because not sure if you guys want this in or maybe want to do your own version; let me know what you think
https://github.com/gabrielguerrero/platform/blob/signal-store-feature-receive-previous-input/modules/signals/src/signal-store-feature.ts

The main changes are in signal-store-feature.ts, and a small change in signal-store-models.ts to MergeTwoFeatureResults, let me know any questions.

[SonyStone]

I needed this feature too, so I created a PR

[gabrielguerrero]

I liked @rainerhahnekamp idea, it handles infinite composition, plus simplifies the types. I can not see any cons of this approach, besides it being a breaking change if it is decided the old way will be deprecated (both could be supported, but I think is better to stick with one).
I'm not sure about the utility functions, I prefer to have one way of doing things if possible via the add( withFeature ) , plus it makes the builder dependent on withState, withMethods, withComputed which I don't like much either

[SonyStone]

Can we add those types first?
#4315
And then do any breaking changes?

[gabrielguerrero]

Yeah I can see the builder as an extra way of doing things for more complex scenarios. I am just not sure if is worth adding an extra api instead of just adding more params to the current way, especially because I think TS will eventually have a nice way of implementing the infinite params, I tried to implement it but there are some limitations currently with using infer with types that have generics that makes it lose types.

[markostanimirovic]

Thanks everyone for sharing your thoughts! This issue will be considered.

#4315 (comment)

[SonyStone]

Can we make the parameters with features for signalStore just an array?

signalStore(conf, [
  withState(),
  withMethods(),
  withMethods(),
  [
    withMethods(),
    withMethods(),
  ]
])

And inside signalStore just do:

function signalStore(config: SignalStoreConfig, args: SignalStoreFeature[]) {
  const features = args.flat();
  ...
}

Then it will be possible to group fixtures into arrays and pass them to signalStore

[markostanimirovic]

Can we make the parameters with features for signalStore just an array?

signalStore(conf, [
  withState(),
  withMethods(),
  withMethods(),
  [
    withMethods(),
    withMethods(),
  ]
])

And inside signalStore just do:

function signalStore(config: SignalStoreConfig, args: SignalStoreFeature[]) {
  const features = args.flat();
  ...
}

Then it will be possible to group fixtures into arrays and pass them to signalStore

With existing TypeScript features, it's not possible to provide strong typing for this.

[rainerhahnekamp]

@SonyStone the closest thing to your array proposal is the builder pattern.

[SonyStone]

@rainerhahnekamp With builder pattern I don't know how to group features together and the builder pattern isn't tree-shakable.

Maybe some kind of Observable.pipe?
signalStore already have signalStoreFeature, вut it does not have an type input through which it knows information about previous features. Is it passable to add it?

signalStore(
  withState()
  withComputed()
  signalStoreFeature(
    withMethods((store) => ...) // ⇽ cannot get value from `withComputed`
  )
)

[rainerhahnekamp]

I don't see the need for adding a grouping feature, but if you find it necessary, we could add it to the builder.

In terms of tree-shaking: If it is only about the add and build methods (as @gabrielguerrero suggested), there is nothing left to shake. You need both methods.