Using Capacitor storage for loading object, but API is still called

Hello,
I am creating rocket launch app using API and I am trying to implement capacitor storage to store objects of launch data that are loaded when launch detail page is viewed. My problem is, that even though object is in storage, API still gets called.

  id = this.route.snapshot.paramMap.get("id");
  inStorage: boolean = false;

  constructor(private apiService: LaunchApiService, private route: ActivatedRoute) {

    this.checkKeys();

    if (this.inStorage) {
      this.getLaunch();
    }
    else {
      this.launch$ = this.apiService.getLaunch$(this.id);
      this.apiService.getLaunch$(this.id).subscribe(data => {
        this.launch = data;       
      })
    }
  }
  async checkKeys() {
    const keys = (await Storage.keys()).keys;
    console.log(keys);

    if (keys.includes(this.id)) {
      this.inStorage = true;
    } else {
      this.inStorage = false;
    }
    console.log("In storage: " + this.inStorage);
  }
  // Storage
  async setLaunch(data) {
    await Storage.set({
      key: this.id,
      value: JSON.stringify(data),
    });
  }

  async getLaunch() {
    const { value } = await Storage.get({
      key: this.id
    });
    this.launch = JSON.parse(value);
  }

In the constructor is not waiting for the outcome before proceeding to the next line. So u need to await the completion.

1 Like

This is a (rare) situation where I disagree with @Tommertom.

First thing: I can’t tell from what you posted whether this code comes from a page or a service. If it’s a page, get it out of there. Pages should not contain data storage implementation details. All of that needs to be in services.

Secondly: I would estimate that roughly 80% of the race conditions I see posted here fall into the following basic script:

  • You have a naked scalar property somewhere (inStorage here)
  • It is set implicitly by an asynchronous process (checkKeys)
  • Some other unrelated code depends on it (constructor)

Avoiding race conditions like this (which are more or less impervious to static code analysis and pretty difficult for humans to spot in manual code review, especially when those three elements are split up across different source files) is the #1 reason that I recommend attempting to avoid modifying external state from asynchronous code as much as possible. When functional programming conventions are followed, you never have to worry about bugs like this.

So instead of compounding the problem with even more opaque syntactic sugar by adding yet another await, I would instead suggest rearchitecting the design to eliminate inStorage completely. I would modify getLaunch and put all the storage/API interaction logic in there. That squashes the race condition, because from the outside, getLaunch gives us an Observable<WhateverALaunchIs>. We don’t know or care where it came from, so we can’t mess up the timing.

1 Like

Fully agree. Just too much going indeed and rethinking is surely more sustainable