Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

service from documentation creates async bug at startup #229

Open
rtpHarry opened this issue Apr 25, 2021 · 6 comments
Open

service from documentation creates async bug at startup #229

rtpHarry opened this issue Apr 25, 2021 · 6 comments

Comments

@rtpHarry
Copy link
Contributor

From my testing of the example service in the documentation, it seems that it contains a bug.

The constructor calls init() without async (because it's not supported in a constructor):

  constructor(private storage: Storage) {
    this.init();
  }

However, that means that if your app is relying on the storage very early on, you are going to get back undefined from a get.

For me, I have a tutorial guard set up which checks if the tutorial has been completed to decide if it should show the homepage or redirect to the tutorial slider page. It was always showing the tutorial because the result was undefined.

I've improved the service so that it takes a similar approach to the way that Ionic Storage is actually written; which is checking if the storage is initialised before trying to operate on it.

That way it can await the proper completion of the storage:

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

import * as CordovaSQLiteDriver from 'localforage-cordovasqlitedriver';

import { Storage } from '@ionic/storage-angular';

@Injectable({
  providedIn: 'root'
})
export class StorageService {
  private _storage: Storage | null = null;

  constructor(private storage: Storage) {
  }

  async init() {
    if(this._storage != null) {
      return;
    }
    await this.storage.defineDriver(CordovaSQLiteDriver);
    const storage = await this.storage.create();
    this._storage = storage;
  }

  public async set(key: string, value: any): Promise<any> {
    await this.init();
    return await this._storage?.set(key, value);
  }

  public async get(key: string): Promise<any> {
    await this.init();
    return await this._storage?.get(key);
  }
}
rtpHarry added a commit to rtpHarry/ionic-storage that referenced this issue Apr 25, 2021
Assuming my ticket, ionic-team#229, is a good solution to the issue I raised, this updates the documentation to use the new technique.
@vishnutdeepak
Copy link

Thank you so much. This was driving me crazy. Please, someone update this in the documentation!

@brianstanley
Copy link

Cool!!

@806software
Copy link

Thanks for pointing this out, as it was driving me nuts too with services relying on storage and it just wasn't working out. I thought I was missing something fundamental. It looks familiar, as basically just you have to create your own ready method ...

public async set(key: string, value: any): Promise<any> {
    await this.init();
    return await this._storage?.set(key, value);
 }

is just the same as:

public set(key: string, value: any): Promise<any> {
    this.init().then(() => {
        return this._storage?.set(key, value);
    }
}

For init read ready.

@dcxn
Copy link

dcxn commented Jun 15, 2021

We had the same issue, thank you @rtpHarry. Hopefully the improvement to the docs can be merged soon.

@aristotekean
Copy link

This solution works for me

From my testing of the example service in the documentation, it seems that it contains a bug.

The constructor calls init() without async (because it's not supported in a constructor):

  constructor(private storage: Storage) {
    this.init();
  }

However, that means that if your app is relying on the storage very early on, you are going to get back undefined from a get.

For me, I have a tutorial guard set up which checks if the tutorial has been completed to decide if it should show the homepage or redirect to the tutorial slider page. It was always showing the tutorial because the result was undefined.

I've improved the service so that it takes a similar approach to the way that Ionic Storage is actually written; which is checking if the storage is initialised before trying to operate on it.

That way it can await the proper completion of the storage:

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

import * as CordovaSQLiteDriver from 'localforage-cordovasqlitedriver';

import { Storage } from '@ionic/storage-angular';

@Injectable({
  providedIn: 'root'
})
export class StorageService {
  private _storage: Storage | null = null;

  constructor(private storage: Storage) {
  }

  async init() {
    if(this._storage != null) {
      return;
    }
    await this.storage.defineDriver(CordovaSQLiteDriver);
    const storage = await this.storage.create();
    this._storage = storage;
  }

  public async set(key: string, value: any): Promise<any> {
    await this.init();
    return await this._storage?.set(key, value);
  }

  public async get(key: string): Promise<any> {
    await this.init();
    return await this._storage?.get(key);
  }
}

arielhasidim added a commit to arielhasidim/ionic-storage that referenced this issue Dec 11, 2021
Fixes "exposing storage service example" from ionic-team#229
@kararkraj
Copy link

kararkraj commented Jun 28, 2024

@rtpHarry

Your solution is working fine for now.
However, storage.create() might get called multiple times depending on the application.

In my case, I have storage.get() in app.component.ts and auth.guard.ts and both are called on application startup.
Hence, storage.create() is called and executed twice.

For now, I do not see any consequence of this.
But I have a feeling that this is not right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants