platform icon indicating copy to clipboard operation
platform copied to clipboard

allow component inputs to be readonly within signal store

Open ducin opened this issue 1 year ago • 3 comments

Which @ngrx/* package(s) are relevant/related to the feature request?

signals

Information

The issue is inspired by this tweet. In short:

  • the OP has a component input() signal which they want signal store to react to
  • the OP found out there are existing 2 ways to do that:
    • effect / allowSignalWrites: true
    • rxMethod
  • in this very case, loadProductDetail seems to be an asynchronous operation, but let's abstract it away, let's assume it could be a synchronous one (which boils down to computed, essentially).

It bothers me, that ngrx signal store is signal-based first and foremost, and that the only 2 solutions are:

  • rxMethod-based -> signals themselves are capable of full-blown (synchronous) reactivity tithout the need of rx
  • effect/allowSignalWrites-based -> allowSignalWrites is both risky and developerPreview (effect, entirely, as for v17), my gut feeling we shouldn't have to reach out for it in such a simple usecase. Effect/signalWrites is a hack, especially when the component input already exists and is a building block that should be possible to compose further.

(in other words we could say that ngrx signal store is incapable of composing component inputs as for now)

I don't think the above would be the most fundamental usecase, though I think combining component Inputs and signal store would be quite common in the long run.

THE IDEA

What I'm thinking of is a way for signal store to include a computed which would be accepted from outside, e.g. apart from withComputed, there'd be withInputs (just made up the name).

This doesn't seem trivial from the API surface area, i.e.:

  • when injecting via inject(MyStore) there's no "syntactic place" to put additional parameters (which could include the hypothetical input signals). Moreover, store could be injected into e.g. services which don't have input signals
  • publishing an additional method on the store class e.g. MyStore.withInputs(inputs) seem to be even worse, because of breaking the well-designed composability which ngrx signal store achieved.
  • hypothetically a new injecting function injectStore could accept additional parameters, e.g. injectStore(StoreClass, { PARAMS }) where one of the params could be simply inputs (and uses native inject underneath). But that, unfortunately, adds another building block to the whole ecosystem which would be nice to avoid.

I'd be happy to provide a PR but, definitely, design work needs to be done first.

EXAMPLE SCENARIO

Let's say our scenario is:

  • the signal store includes it's own state, e.g. a collection of objects
  • the component has an input() signal which passes the chosen IDs which we want to filter and do some calculations on top of them
  • we want the store to do the (optional) filtering + calculations, e.g. within withComputed
  • whenever the chosen IDs change, store reacts to it. The store could accept a readonly signal and make the withComputed consume it/depend on it

last note

if anything is anyhow unclear I'm happy to explain further.

Again, as the component input() signal already exists and can be used, it feels wrong to me to either use effect/signalWrites or rxMethod.

Describe any alternatives/workarounds you're currently using

No response

I would be willing to submit a PR to fix this issue

  • [X] Yes
  • [ ] No

ducin avatar May 10 '24 11:05 ducin

Some time ago, @alex-okrushko shared an example of how to implement the withInputBindings feature:

with-input-bindings

markostanimirovic avatar May 10 '24 17:05 markostanimirovic

Okay, so that was through an external function. @markostanimirovic I think it'd make sense to include that at least in the docs - I could make a PR to signal store docs. WDYT?

ducin avatar May 10 '24 18:05 ducin

Hey @ducin @markostanimirovic, in my tweet, my case was calling the backend, so if I wanted to do that with this solution, it would require a withHook and onInit that listens to the stored signals to trigger the call; I still think for my case, the rxMethod that you pass the signal is easier. But I really like this solution for doing computed of signal inputs, I think might be a good candidate for a core feature even or something similar because it is a very common case as well. At least, like you mentioned, I think something in docs explaining how to pass input signals to the store will be great because everyone will face that case at some point. I could add that feature to @ngrx-traits/signals not sure if you have seen that package but Im trying to build a set of common util store features package, if @alex-okrushko doesnt mind :).

gabrielguerrero avatar May 10 '24 19:05 gabrielguerrero

I'm going to convert this issue into a discussion because we don't plan to add it to the core package, at least for now.

@gabrielguerrero Sure, it would be great to have a feature like this in a community plugin!

markostanimirovic avatar Jul 19 '24 23:07 markostanimirovic