1

I am developing an Angular Application as hobby project and trying to write clean code with best practices.

I created a ".loader" css class and put it on the main div of app-component. My purpose is to attach this class when the BackendService.loading field is true and detach when false. This will create a loading effect for the whole page. I planned to change this BackendService.loading to true when I call an endpoint and to false when response is provided. I crate this BackendService to handle all other services' http calls. So that I don't have to set this variable in every components in every subscription's response callback.

The problem: I can set variable from BackendService when a new api call request came but I could not set it to false again when it is end because it's subscription is done in component controller.

How can I set the loading variable in BackendService to false when the response is provided? (I read too much things about Observable and tried to wrap http.get's return in another observable for example but none of them is worked) Or do I need to implement something else which is more clean?

BackendService:

export class BackendService {
  apiUrl = 'http://localhost:8080';

  loading = false;

  token = null;

  constructor(private http: HttpClient) {}

  getOptions() {
    return {
      headers: new HttpHeaders()
        .set('Authorization', this.token)
        .set('Content-type', 'application/json')
        .set('Access-Control-Allow-Origin', '*')
    };
  }

  getRequest(path: string): Observable<any> {
    return this.http.get(this.apiUrl + path, this.getOptions());
  }

  postRequest(path: string, body: Object): Observable<any>  {
    return this.http.post(this.apiUrl + path, body, this.getOptions());
  }
}

Random other service which use BackendService to make api calls:

export class StudentService {

  constructor(private backendService: BackendService) {}

  updateLevel(studentId: number) {
    return this.backendService.getRequest(
      '/student/update-level/' + studentId + '/6/turkce/paragraf');
  }

  getSummary(studentId: number) {
    return this.backendService.getRequest(
      '/student/summary/' + studentId) as Observable<StudentSummaryModel>;
  }
}

An api call inside a component controller: (Here I have to inject BackendService.loading in result callback)

export class QuizStartComponent implements OnInit {

  constructor(private router: Router,
              private quizService: QuizService,
              private studentService: StudentService,
              private userService: UserService) { }

  ngOnInit() {
  }

  quizStart(quizSize: number) {
    this.quizService.quizSize = quizSize;

    this.studentService.updateLevel(this.userService.user.id).subscribe(
      res => {
        this.router.navigate(['/student/question']);
      }, err => {

      });

  }
}

Not related but these are app.component.ts and loader css class:

<app-navigation></app-navigation>
<div class="container mb-auto" [ngClass]="{'loading': backendService.loading}">
  <div class="mt-3">
    <router-outlet></router-outlet>
  </div>
</div>
<app-footer></app-footer>

.loading {
  ... some css
}

.loading:after {
  ... some css
}
H.Cagatay
  • 11
  • 1
  • Its usually a matter of single bool flag and *ngIf on spinner component. – Antoniossss Nov 16 '19 at 18:11
  • Actually, the problem in here is how and where to set that single bool flag. Do we need to set it for every call or can we make it more generic to keep the code clean. @Antoniossss – H.Cagatay Nov 18 '19 at 15:20
  • adding 1 liner of code to every code of clear intention seems quite clear to me. – Antoniossss Nov 18 '19 at 20:46

2 Answers2

1

There are many opinions on how to do a loader so I won't dive into that. As for doing something after every request completes, All you should have to do is pipe() it out and use tap():

  getRequest(path: string): Observable<any> {
    this.loading = true;
    return this.http.get(this.apiUrl + path, this.getOptions()).pipe(tap(() => this.loading = false));
  }

https://www.learnrxjs.io/operators/utility/do.html

Damian C
  • 2,111
  • 2
  • 15
  • 17
  • Thank you it worked! It caused to ExpressionChangedAfterItHasBeenCheckedError but it could easly be solved as explained in this post: https://stackoverflow.com/a/46815744/12383396 – H.Cagatay Nov 16 '19 at 17:16
  • As far as I can tell this will not work in case of error. Loading will not be falsified. – Antoniossss Nov 18 '19 at 20:46
1

I've usually found it useful to define a service that I can inject anywhere I need to show a spinner or set some loading state for the app.

A simple service to tell the app to set the loading state or maybe a spinner:

@Injectable()
export class LoadingService {
    loading$ = new Subject<Boolean>();

    appLoading(isLoading: boolean){ this.loading$.next(isLoading); )
}

Then in some other service that manages some business logic:

@Injectable()
export class BusinessLogicService {
    constructor(private http: HttpClient, private loadingService: LoadingService){}

    startQuiz(quizId: number): Observable<any> {
       this.loadingService.appLoading(true);
       return this.http.put('quizes' + quizId + '/start', {})
           .pipe(tap( () => {
                //request succeeded, it's no longer loading.
                this.loadingService.appLoading(false);
            }));
           .pipe(catchError(err => {      
                //request failed, it's not loading and let any downstream error handling occur
                this.spinner.hide();
                throw err;
            }));
    }    
}

Then your app component can subscribe to the loading state:

<app-navigation></app-navigation>
<div class="container mb-auto" [ngClass]="{'loading': isLoading}">
  <div class="mt-3">
    <router-outlet></router-outlet>
  </div>
</div>
<app-footer></app-footer>


export class AppComponent implements OnInit, OnDestroy {
   isLoading = false;
   loadingSub: Subscription;

   constructor(private loadingService: LoadingService){}

   ngOnInit(){
       this.loadingSub = this.loadingService.loading$.subscribe(isLoading => {
           this.isLoading= isLoading;
       });
   }

   ngOnDestroy(){
      //unsub to avoid leaks
      this.loadingSub.unsubscribe();
   }

}
spots
  • 2,483
  • 5
  • 23
  • 38