Akita: Feature request: Add selectLoading() to non-Entity store/query

[seangwright]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[X] Feature request
[ ] Documentation issue or request
[ ] Support request
[ ] Other... Please describe:

Current behavior

Entity queries expose selectLoading() so a component can toggle spinners while an http request is being made.

Queries for non-entities don't have this method and the non-entity stores don't maintain this state.

Expected behavior

Non-entity queries and stores exposing loading state/querying.

What is the motivation / use case for changing the behavior?

When I have sets of entities, entity stores/queries are great. But I often have single items I need to track state for (current session/user, a 'Detail' model of an entity that I'm editing) and I am usually making http requests to get this data from the api.

I would love to be able to query on this loading state for these non-entity collection stores.

I would be willing to help with a PR if you think this feature is worth adding, or if not I'm open to other ways of implementing it.

Thanks.

[NetanelBasal]

This is a great suggestion. We can move the existing API to the basic store. I'm wondering if it's something that we should add permanently in a basic store or maybe we should allow this by passing a flag, something like:

@StoreConfig( { loadingState: true }) { }

@DmitryEfimenko @hoisel @MonsieurMan @shaharkazaz @sgwatgit what do u think guys?

[NetanelBasal]

To maintain consistency, I think it should always be. (It probably will not hurt anyone.)

[seangwright]

My thoughts...

I'm drawn to Akita because it's opinionated but also provides flexibility. I don't need to build everything from scratch like with NgRx / NgXS / NgRx-data - if I had the need for maximum flexibility I would be using one of those libraries.

isLoading is something I've used in every Angular project I've worked on, either exposing it with a component property as a boolean or Observable<boolean>. Spinners are inherit to the web because asynchronicity is and the ability to toggle a spinner on/off is a requirement to using them and they always toggle based on a loading state.

It's not so much a question of is isLoading useful enough to be baked into Akita, but more a question of how to implement it for the best developer ergonomics.

I would recommend it is added in the core (all stores) and if you want to be able to turn it off, make it _opt-out_ rather than _opt-in_.

This is what I'm currently doing to implement this behavior in a basic store. You can see how it's not too difficult to expose the data in the store separate from the loading state. If developers are not interested in isLoading then they could work only with the entity state apis of Akita.

import { Injectable } from '@angular/core';

import { Store, StoreConfig } from '@datorama/akita';

import { User } from '@app/shared/user/user.model';

export interface CurrentUser extends User {}

export interface CurrentUserState {
  currentUser: CurrentUser;
  isLoading: boolean;
}

export function createCurrentUser(
  currentUser?: Partial<CurrentUser>,
): CurrentUserState {
  return {
    currentUser: { ...currentUser },
    isLoading: false,
  } as any;
}

@Injectable({ providedIn: 'root' })
@StoreConfig({ name: 'current-user' })
export class CurrentUserStore extends Store<CurrentUserState> {
  constructor() {
    super(createCurrentUser({}));
  }
}

And this is what my query looks like

import { Injectable } from '@angular/core';

import { Query } from '@datorama/akita';

import {
    CurrentUserState, CurrentUserStore
} from '@app/core/authentication/state/current-user.store';

@Injectable({ providedIn: 'root' })
export class CurrentUserQuery extends Query<CurrentUserState> {
  constructor(protected store: CurrentUserStore) {
    super(store);
  }

  selectCurrentUser(selectOnce: boolean = false) {
    return selectOnce
      ? this.selectOnce(s => s.currentUser)
      : this.select(s => s.currentUser);
  }

  selectIsLoading(selectOnce: boolean = false) {
    return selectOnce
      ? this.selectOnce(s => s.isLoading)
      : this.select(s => s.isLoading);
  }
}

[NetanelBasal]

Yes, I thought more about that, in most cases you need the loading state in each one of the stores. I believe that the basic store should support the loading API as the entity store.

[NetanelBasal]

@sgwatgit I would love to see a PR.

[seangwright]

Ok, I'll take some time this weekend to look at it.

[NetanelBasal]

You don't need to do it, I already did. In order to don't introduce any breaking changes, the only thing that the consumer needs to do is to add the loading key, and he will get the setLoading() and selectLoading() methods. I also added the error functionality to the basic store.

Will merge soon. Thanks.

[seangwright]

Darn, I was hoping to see my picture on the README 😅 .

Well that's awesome. I'll be updating whenever it's released. Akita is fitting very well into a new project I'm architecting.

Thanks!

[DmitryEfimenko]

I agree. This should be a part of the base store. Great job getting this in Akita!

However, I'd also like to point out that a spinner is usually a component that's shown during various HTTP requests. These HTTP requests may update completely different Akita stores, so it's nicer to have a single LoadingService for the purpose of the global spinner.

I also usually prefer not to have that logic in the WhateverAkitaStoreSerive, but rather have it handled by an interceptor like in this answer on SO.

Having said that, I'm sure there will be use cases when it will be useful to have this logic baked in the Akita's base store.

[seangwright]

I have worked on apps with both global spinners (like the loading bar in ngx-datatable https://swimlane.github.io/ngx-datatable/) that visually reflect a loading state and also localized spinners that only display a loading state on part of the screen since multiple http requests can be in flight for different purposes.

If you want to treat all store loading observables as one global loading state then an interceptor is a fine cross-cutting option but in an app with a complex UI representing different entities I've often needed to show progress or a loading indicator that is relevant to a given entity.

The store is a perfect place to centralize state for the above scenario - and if you don't need it, it seems like @NetanelBasal has designed it to not show up.

There's probably a design pattern to parse the entity type out of a request in an interceptor and update a store with a state of loading for that type, then query on that store for the loading of a given type (a string union would be helpful here).

Hmm... maybe this approach (a separate store just for loading, wired into all your data services or an interceptor) would be better for SRP?

[NetanelBasal]

@sgwatgit your name will be there anyway with the "ideas" badge. Thanks for the idea.

[NetanelBasal]

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

The release is available on npm package (@latest dist-tag)

Your semantic-release bot :package::rocket:

[NetanelBasal]

but in an app with a complex UI representing different entities I've often needed to show progress or a loading indicator that is relevant to a given entity.

@DmitryEfimenko he is right. But if in your application you always have one spinner, I definitely would go with the interceptor solution and not using Akita store for it.