ngrx / platform

Reactive State for Angular
https://ngrx.io
Other
8.01k stars 1.97k forks source link

@ngrx/data pr #3228 broke the loaded flag without introducing an alternative #4025

Open VFiber opened 1 year ago

VFiber commented 1 year ago

Which @ngrx/* package(s) are the source of the bug?

data

Minimal reproduction of the bug/regression with instructions

Before #3228 @ngrx/data provided an option to query if the local Entity-cache already contains a snapshot of every entity trough the ˙loaded flag.

Although the naming was a bit off, after #3228 there is no option to determine if every entity is loaded or if the store only contains partial data.

What makes this as a bug: Since V8 (ngrx/data introduced) this property was not changed in the documentation (see: https://v8.ngrx.io/guide/data/entity-collection, https://ngrx.io/guide/data/entity-collection (v16)).

Expected behavior

loaded flag only gets true when QueryAll successfully executed and the data patched back to the state.

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s)

NgRx: 13.0

Other information

My suggested alternative solution is:

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

VFiber commented 1 year ago

After trying to work around the change found some additional logical issues with it.

If it is a flag for successful load why it does affect QUERY_MANY_SUCCESS, and not QUERY_BY_KEY_SUCCESS?

With the new version in mind, I would assume it is a flag and applies to every QUERY_*_SUCCESS Action when data has been successfully retrieved from the API.

martijnMedialen commented 6 months ago

please fix this!

Maybe also add an option to set loaded state because i have a usecases where I want to load a subset of my data (only loading active users and want to have an option to load all later) and have the previous workaround build in.