search

angular-architects / ngrx-toolkit

Various Extensions for the NgRx Signal Store
MIT License
98 stars 16 forks source link

Issues with the `withCallState` addon #18

Closed antimprisacaru closed 4 months ago

antimprisacaru commented 4 months ago

Hey there!

After playing around with the withCallState addon to the SignalStore, I've wound up in a situation where I wanted the call state for multiple subslices of my state. I've found out that in order to set loading, error or loaded, I'd have to use the callback implementation of the patchState, as patching directly would throw errors that "NamedCallStateSlice<'myState'> | CallStateSlice" would not be assignable to the updater (defined as Array<Partial<State & {}> | PartialStateUpdater<State & {}>>). By converting to callback updater, I was able make it work like this:

patchState(store, (state) => ({ ...state, ...setLoading(state) }))

I think that this DX is pretty cumbersome to see in code, hence why I suggest improving it. Perhaps further investigating the mismatch between the result of setLoading and others, or making a factory that exports these functions and the call state with the name in them already. These are just some high level things, I haven't really looked very deeply into what's causing that mismatch, seeing that the unnamed call state doesn't have any issues.

Best regards!

rainerhahnekamp commented 4 months ago

Hi Antim, thanks for letting us know. You know how it is with Open Source projects ;) if you could contribute and improve the current state, that would be great and speed up things.

Maybe you want to connect with @andersfgraneng. He has opened https://github.com/angular-architects/ngrx-toolkit/pull/16

andersfgraneng commented 4 months ago

I think we encountered the same issue. I think the PR mentioned by rainerhahnekamp will solve it. You should then be able to use patchState(store, setLoading('myState')) without typescript complaining. Its because the function return type is as you mentioned: NamedCallStateSlice<'myState'> | CallStateSlice, which can be narrowed down based on the parameter. If you coincidentally use withCallState() and withCallState('myState') when defining your store typescript wont complain, since then your store satisfies the return types. You could try to pull the changes and see if it solves your issue?

antimprisacaru commented 4 months ago

I think we encountered the same issue. I think the PR mentioned by rainerhahnekamp will solve it. You should then be able to use patchState(store, setLoading('myState')) without typescript complaining. Its because the function return type is as you mentioned: NamedCallStateSlice<'myState'> | CallStateSlice, which can be narrowed down based on the parameter. If you coincidentally use withCallState() and withCallState('myState') when defining your store typescript wont complain, since then your store satisfies the return types. You could try to pull the changes and see if it solves your issue?

Hi Anders, pulled it and gave it a try. Everything seems to work fine now. Great work :)

I'd also add some tests to the call state for both normal and named scenarios perhaps, since this bit doesn't have any.

rainerhahnekamp commented 4 months ago

This issue has been fixed thanks to @andersfgraneng and is available in version 0.0.7.

@antimprisacaru I've added some basic tests, but if you have some other tests to add, that would be great.