search

gabrielguerrero / ngrx-traits

NGRX Traits is a library to help you compose and reuse state logic in your angular app. There is two versions, @ngrx-traits/signals supports ngrx-signals, and @ngrx-traits/{core, common} supports ngrx.
MIT License
44 stars 3 forks source link

withCalls should optionally allow returning reactive values #102

Closed Toeler closed 4 days ago

Toeler commented 3 weeks ago

Currently, withCalls will only return the first value emitted, and will then unsubscribe from the source (https://github.com/gabrielguerrero/ngrx-traits/blob/main/libs/ngrx-traits/signals/src/lib/with-calls/with-calls.ts#L258).

I have a need to return a value which can emit multiple times. I don't need loading states for each, as the value will just sometimes emit more up-to-date values. I just need the first loading state for the initial fetch.

I currently have a workaround using withMethods and withCallStatus, but it would be nicer to be able to configure (opt-in) withCalls to not limit to just 1 emitted value.

Reproduction: https://stackblitz.com/edit/stackblitz-starters-1wjxgd?file=src%2Fmain.ts

I am happy to submit a PR if this is something that you are interested in the library supporting.

gabrielguerrero commented 3 weeks ago

Hey @Toeler , I think is a good use case, and yes you could submit a PR, the only thing Im thinking maybe the first is not needed at all, because normal httpClient calls only return one value, calls that return promise only return one value, so Im trying to think if just removing the first could be a breaking change.

gabrielguerrero commented 2 weeks ago

Hey @Toeler, Im almost convinced about removing the first, I can't think yet of a normal case this could break, but can you tell me more about your use case for example are you connecting to WebSocket that is emitting periodically new values?

Toeler commented 2 weeks ago

Hi @gabrielguerrero. My use case is that I have a liveQuery to a Dexie database. The initial request is quick, but not instant, so I still want to be able to track the call state. But once we have a result, then any changes that are pushed to IndexedDB will result in the observable emitting a new value.

My initial thought was to make a non-breaking change by making the new behaviour opt-in. But if you are willing to do a version bump with a new breaking change then it is even easier to implement. If the consumer ever needed a single response but had an open observable then they can simply add a first() or take(1) to the observable chain that they passed in.

gabrielguerrero commented 2 weeks ago

Hey @Toeler, I want to proceed and remove the first and maybe add a takeUntilDestroy, just in case you still want to do a PR?; you might break some tests, but you can contact me in Discord if you need any help. Otherwise I can do the change.

Toeler commented 1 week ago

I'm happy to make the change @gabrielguerrero. Give me a day or two to get the repo pulled down and the tests running and I'll make the changes and have a PR up later this week!

gabrielguerrero commented 1 week ago

@Toeler excellent let me know any problem

github-actions[bot] commented 1 week ago

:tada: This issue has been resolved in version 17.9.0-beta.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

github-actions[bot] commented 4 days ago

:tada: This issue has been resolved in version 17.9.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: