Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow component inputs to be readonly within signal store #4323

Open
1 of 2 tasks
ducin opened this issue May 10, 2024 · 3 comments
Open
1 of 2 tasks

allow component inputs to be readonly within signal store #4323

ducin opened this issue May 10, 2024 · 3 comments

Comments

@ducin
Copy link
Contributor

ducin commented May 10, 2024

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

  • Yes
  • No
@markostanimirovic
Copy link
Member

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

with-input-bindings

@ducin
Copy link
Contributor Author

ducin commented May 10, 2024

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?

@gabrielguerrero
Copy link
Contributor

gabrielguerrero commented May 10, 2024

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 :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants