1

As an amateur PHP coder, I'm having a hard time grasping JS async behavior. I'm writing a Firefox WebExtension that uses two StorageAreas from the Storage API for setting/getting options, and I want to merge all options into one object to pass around the application efficiently:

'use strict';
class Options {

  constructor() {
    this.defaults = {};
    this.locals = {};
    this.options = {};
  }

  getDefaults() {
    browser.storage.managed.get().then(res => {
      this.defaults = res;
    });
  }

  getLocals() {
    browser.storage.local.get().then(res => {
      this.locals = res;
    });
  }

  get() {
    this.getDefaults();
    this.getLocals();
    this.options = Object.assign(this.defaults, this.locals); // <-- This is where the code fails: this.defaults and this.locals are empty.
    return this;
  }
}

const opts = new Options().get();
console.log(opts); // Object { defaults: {}, locals: {}, options: {} } background.js:31:1
console.log(Object.keys(opts)); // Array(3) [ "defaults", "locals", "options" ] background.js:32:1
console.log(opts.defaults); // Object {  } background.js:33:1

const options = Object.assign(opts.defaults, opts.locals);
console.log(options); // Object {  } background.js:36:1

I have indicated the line where the error first triggers, and after banging my head against the same wall for 2+ days now, I believe it is either related to the async character of the Promise returned by Firefox's browser.storage.*.get(), or related to variable scopes.

I have tried:

  1. declaring a local variable in the get*-functions (let that = this;)
  2. using async/await in the get*-functions
  3. binding the get*-functions' result to either this or this.defaults
  4. before creating this class, I started with nested Promises, but also then I was unsuccesful in creating a (global) 'options' variable.

Thx for any pointers - my mind is tired of reviewing/rewriting these 36 loc...

zenlord
  • 330
  • 3
  • 15
  • Yes, you will eventually need to nest promises, you cannot create a global variable that will immediately have the option values. – Bergi Mar 17 '19 at 19:36

2 Answers2

1

You do not wait for the promises create by the calls this.getDefaults(); and this.getLocals(); so at the time you do this.options = Object.assign( ... the data is not ready.

If you create a Promise in a function then you need to return from it so that the caller can wait for that Promise, to resolve.

getDefaults() {
  return browser.storage.managed.get().then(res => {
    this.defaults = res;
  });
}

getLocals() {
  return browser.storage.local.get().then(res => {
    this.locals = res;
  });
}

get() {
    return Promise.all([
      this.getDefaults(),
      this.getLocals()
    ])
    .then(() => {
       this.options = Object.assign(this.defaults, this.locals);
       return this
    })
})

And you for sure also need to use then or await on get() to wait for get to be finished.

t.niese
  • 39,256
  • 9
  • 74
  • 101
  • Thank you so much - indeed, now the Object.assign() works as expected and all three properties are correctly set. The code however still fails to log the value of opts.defaults or opts.options to console.I believe you are hinting to the solution with your last sentence, but I already tried multiple possible solutions to no avail... (adding .then() to this.get() / adding .finally() to this.get() / adding .then() to new Options().get()), and returning from different positions within this.get(). Please advise? – zenlord Mar 17 '19 at 18:35
  • Yes @t.niese meant to use like opts.getDefaults().then( value => { /* use these to update the class properties and then return this */ }) – Nitin Kumar Mar 17 '19 at 18:57
  • And I think, you have to use await opts.get() to receive the values – Nitin Kumar Mar 17 '19 at 18:58
  • 1
    @zenlord Writing `new Options().get().then(opts => {console.dir(opts})` would show the the Options objects with all values set. You really should take some time to learn the fundamentals of Promises and `await`/`async` to that you understand what Promises are and how they work. – t.niese Mar 17 '19 at 19:01
1

Don't write asynchronous functions to initialise your instances. Instead, do the asynchronous work before constructing the objects, let the constructor take the data as parameters.

In your case, that's

class Options {
  constructor(defaults = {}, locals = {}) {
    this.defaults = defaults;
    this.locals = locals;
    this.options = Object.assign(this.defaults, this.locals);
  }

  // static methods that don't modify an instance, but return promises
  static getDefaults() {
    return browser.storage.managed.get();
  }
  static getLocals() {
    return browser.storage.local.get();
  }

  // static method to load data and create an instance, returning a promise for it
  static async get() {
    const [defaults, locals] = await Promise.all([this.getDefaults(), this.getLocals()]);
    return new this(defaults, locals);;
  }
}

const options = Options.get();
console.log(options); // Promise {  }
options.then(opts => {
  console.log(opts); // Options { defaults: {…}, locals: {…}, options: {…} }
  console.log(Object.keys(opts)); // Array(3) [ "defaults", "locals", "options" ]
  console.log(opts.defaults); // Object { … }
});
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • 1
    wonderful! so basically, you need to call the static method that creates an instance of the class by passing to it the needed (resolved) parameters. – umbe1987 Apr 23 '20 at 16:20