Is It Bad Practice to Have a Constructor Function Return a Promise

Is it bad practice to have a constructor function return a Promise?

Yes, it is a bad practise. A constructor should return an instance of its class, nothing else. It would mess up the new operator and inheritance otherwise.

Moreover, a constructor should only create and initialize a new instance. It should set up data structures and all instance-specific properties, but not execute any tasks. It should be a pure function without side effects if possible, with all the benefits that has.

What if I want to execute things from my constructor?

That should go in a method of your class. You want to mutate global state? Then call that procedure explicitly, not as a side effect of generating an object. This call can go right after the instantiation:

var engine = new Engine()
engine.displayPosts();

If that task is asynchronous, you can now easily return a promise for its results from the method, to easily wait until it is finished.

I would however not recommend this pattern when the method (asynchronously) mutates the instance and other methods depend on that, as that would lead to them being required to wait (become async even if they're actually synchronous) and you'd quickly have some internal queue management going on. Do not code instances to exist but be actually unusable.

What if I want to load data into my instance asynchronously?

Ask yourself: Do you actually need the instance without the data? Could you use it somehow?

If the answer to that is No, then you should not create it before you have the data. Make the data ifself a parameter to your constructor, instead of telling the constructor how to fetch the data (or passing a promise for the data).

Then, use a static method to load the data, from which you return a promise. Then chain a call that wraps the data in a new instance on that:

Engine.load({path: '/path/to/posts'}).then(function(posts) {
    new Engine(posts).displayPosts();
});

This allows much greater flexibility in the ways to acquire the data, and simplifies the constructor a lot. Similarly, you might write static factory functions that return promises for Engine instances:

Engine.fromPosts = function(options) {
    return ajax(options.path).then(Engine.parsePosts).then(function(posts) {
        return new Engine(posts, options);
    });
};

…

Engine.fromPosts({path: '/path/to/posts'}).then(function(engine) {
    engine.registerWith(framework).then(function(framePage) {
        engine.showPostsOn(framePage);
    });
});

Is passing a Promise as a function argument considered a bad practise?

Yes, one should generally avoid passing promises to functions. Prefer waiting for the promise and passing only the result, i.e. do

somethingAsync().then(x => someFunction(x, y))
// or
someFunction(await somethingAsync(), y)

instead of

someFunction(somethingAsync(), y)

As you say, it's very confusing otherwise, especially when not using TypeScript to tell you when you have messed up. And the implementation of someFunction becomes much simpler when it is synchronous and doesn't have to deal with any asynchronous logic, which also makes it more universally useful.

Of course, there are exceptions to every rule, and they would be functions that are explicitly dedicated to handle asynchronous promise logic, like Promise.resolve, Promise.all, Promise.race, .then() etc. Sometimes you write similar helper functions yourself, e.g. for error-handling, and in those cases it might be acceptable to pass a promise. It's still unusual though.

Asynchronous operations in constructor

It is particularly difficult to put asynchronous operations in a constructor. This is for several reasons:

The constructor needs to return the newly created object so it can't return a promise that would tell you when the async operation is done.
If you do an asynchronous operation inside the constructor that sets some instance data and the constructor returns the object, then you have no way for the calling code to know when the async operation is actually done.

For these reasons, you usually don't want to do an async operation inside a constructor. IMO, the cleanest architecture below is the factory function that returns a promise that resolves to your finished object. You can do as much asynchronous stuff as you want in the factory function (call any methods on the object) and you don't expose the object to the caller until it is fully formed.

These are some of the various options for dealing with that issue:

Use Factory Function that Returns a Promise

This uses a factory function that does some of the more common work for you. It also doesn't reveal the new object until its fully initialized which is a good programming practice as the caller can't accidentally try to use a partially formed object in which the asynchronous stuff hasn't finished yet. The factory function option also cleanly propagates errors (either synchronous or asynchronous) by rejecting the returned promise:

// don't make this class definition public so the constructor is not public
class MyObj() {
   constructor(someValue) {
       this.someProp = someValue;
   }
   init() {
       return Service.getService().then(val => {
          this.asyncProp = val;
          return this;
       });
   }
}

function createMyObj(someValue) {
    let x = new MyObj(someVal);
    return x.init();
}

createMyObj(someVal).then(obj => {
    // obj ready to use and fully initialized here
}).catch(err => {
    // handle error here
});

If you're using modules, you can export only the factory function (no need to export the class itself) and thus enforce that the object is initialized properly and not used until that initialization is done.

Break async object initialization into a separate method that can return a promise

class MyObj() {
   constructor(someValue) {
       this.someProp = someValue;
   }
   init() {
       return Service.getService().then(val => {
          this.asyncProp = val;
       });
   }
}

let x = new MyObj(someVal);
x.init().then(() => {
    // ready to use x here
}).catch(err => {
    // handle error
});

Use Events to Signal Completion

This scheme is used in a lot of I/O related APIs. The general idea is that you return an object from the constructor, but the caller knows that object hasn't really completed its initialization until a particular event occurs.

// object inherits from EventEmitter
class MyObj extends EventEmitter () {
   constructor(someValue) {
       this.someProp = someValue;

       Service.getService().then(val => {
          this.asyncProp = val;
          // signal to caller that object has finished initializing
          this.emit('init', val);
       });
   }
}

let x = new MyObj(someVal);
x.on('init', () => {
    // object is fully initialized now
}).on('error', () => {
    // some error occurred
});

Hackish way to put the Async Operation in the Constructor

Though I wouldn't recommend using this technique, this is what it would take to put the async operation in the actual constructor itself:

class MyObj() {
   constructor(someValue) {
       this.someProp = someValue;
       this.initPromise = Service.getService().then(val => {
          this.asyncProp = val;
       });
   }
}

let x = new MyObj(someVal);
x.initPromise.then(() => {
   // object ready to use now
}).catch(err => {
   // error here
});

Note, you see the first design pattern in many places in various APIs. For example, for a socket connection in node.js, you would see this:

let socket = new net.Socket(...);
socket.connect(port, host, listenerCallback);

The socket is created in the first step, but then connected to something in the second step. And, then the same library has a factory function net.createConnection() which combines those two steps into one function (an illustration of the second design pattern above). The net module examples don't happen to use promises (very few nodejs original apis do), but they accomplish the same logic using callbacks and events.

Other note on your code

You likely also have an issue with the value of this in your code. A .then() handler does not naturally preserve the value of this from the surrounding environment if you pass it a regular function() {} reference. So, in this:

function Constructor(){
   Service.getService().then(function(data){
      this.arr = data.data.array;
      return this.arr
   })
}

The value of this when you try to do this.arr = data.data.array; is not going to be correct. The simplest way to fix that issue in ES6 is to use a fat arrow function instead:

function Constructor(){
   Service.getService().then(data => {
      this.arr = data.data.array;
      return this.arr
   });
}

How to call promise object in constructor to set a property

how to make sure that return happens after promise is resolved?

You can't. The app function will return before the promise is resolved. That is even guaranteed by JavaScript.

How to call promise object in constructor to set a property

You don't.

Instead you have something like a factory method that creates a new instance of the class, and returns a promise that resolves to the instance.

Example:

function getCookie(someParams) {
 return connect(someParams)
   .then(function(response){
     return response.cookie;
   });
}

function App() {}

function getApp() {
  var app = new App();
  return getCookie(params)
    .then(function (cookie) {
      app.cookie = cookie;
      return app;
    });
}

// Usage

getApp().then(function(app) {
  // use app
});

One uses promises for asynchronous processes. Constructor functions are synchronous. While you can use promises in constructors, the returned instance won't be fully initialized until the promise is resolved. But your code would never know when that is.

That's why having a factory method that returns a promise that resolves to the class instance if the more reliable way.

Synchronous code in constructor with Promises

Your objective of not returning the instance until after the connection won't work (at least not like this). The constructor's job is to create an instance and return that. Functions need to return something synchronously. If it's performing some async operation then a function can return a promise instead, but you don't want a promise from the constructor — you need the instance.

The easy way to do this is to require your object to be initialized after it's created, then the constructor can construct and return the instance and the init function is free to return a promise:

class AClass {

  constructor(n) {/* some setup */}

  func() {

    return new Promise(resolve =>  {

      setTimeout(() => {

        resolve("some name");

      }, 1000);

    }); 

  }

  async init() {

    this.name = await this.func();

    return this

  }

}

new AClass('test').init()

.then((initialized_obj) => console.log(initialized_obj))

Handle promise error in class constructor

The Promise is assigned to this.connection, but you're .catching the error in the constructor. Better to catch the error only when you can do something with it - that is, in the consumer of the Service outside. So, just move the .catch from the constructor to right below where you create the service:

const client = require('aService')

class Service {
  constructor() {
    this.connection = client.login()
      .then(() => {
        // ...
      });
  }
}

let s = new Service()
s.connection.catch((e) => {
  console.log(`connection error ${e}`)
});

Or use await with try/catch:

const client = require('aService')

class Service {
  constructor() {
    this.connection = client.login()
      .then(() => {
      // ...
    });
  }
}

(async () => {
  let s = new Service()
  try {
    await s.connection;
    // connection done
  } catch(e) {
    console.log(`connection error ${e}`)
  }
})();

Get a constructor variable in promise

Do not try to use state or make ajax calls in the constructor of your React component. Instead, put that call inside one of the lifecycle methods that fires immediately, like componentWillMount. Also to access this.state inside of your ajax callback, you will need to bind this to the function. Using the fat arrow function syntax is the most straightforward way.

class StockGraph extends Component {
    constructor(props) {
        super(props);
        this.state = { numXLabels: 0 }
    }

    componentWillMount() {
        var url = 'https://www.quandl.com/api/v3/datasets/WIKI/MSFT'+
           '.json?api_key=bCRpjzvgPNkxLzqAv2yY';
        fetch(url)
        .then((response) => {
           return response.json()
         })
        .then((json) => {
            console.log(this.state.numXLabels);
            //this.setState({
             // numXLabels: 30
           //})
        })
    }
    ...

Async/Await Class Constructor

This can never work.

The async keyword allows await to be used in a function marked as async but it also converts that function into a promise generator. So a function marked with async will return a promise. A constructor on the other hand returns the object it is constructing. Thus we have a situation where you want to both return an object and a promise: an impossible situation.

You can only use async/await where you can use promises because they are essentially syntax sugar for promises. You can't use promises in a constructor because a constructor must return the object to be constructed, not a promise.

There are two design patterns to overcome this, both invented before promises were around.

Use of an init() function. This works a bit like jQuery's .ready(). The object you create can only be used inside it's own init or ready function:

Usage:

var myObj = new myClass();
myObj.init(function() {
    // inside here you can use myObj
});

Implementation:

class myClass {
    constructor () {

    }

    init (callback) {
        // do something async and call the callback:
        callback.bind(this)();
    }
}

Use a builder. I've not seen this used much in javascript but this is one of the more common work-arounds in Java when an object needs to be constructed asynchronously. Of course, the builder pattern is used when constructing an object that requires a lot of complicated parameters. Which is exactly the use-case for asynchronous builders. The difference is that an async builder does not return an object but a promise of that object:

Usage:

myClass.build().then(function(myObj) {
    // myObj is returned by the promise, 
    // not by the constructor
    // or builder
});

// with async/await:

async function foo () {
    var myObj = await myClass.build();
}

Implementation:

class myClass {
    constructor (async_param) {
        if (typeof async_param === 'undefined') {
            throw new Error('Cannot be called directly');
        }
    }

    static build () {
        return doSomeAsyncStuff()
           .then(function(async_result){
               return new myClass(async_result);
           });
    }
}

Implementation with async/await:

class myClass {
    constructor (async_param) {
        if (typeof async_param === 'undefined') {
            throw new Error('Cannot be called directly');
        }
    }

    static async build () {
        var async_result = await doSomeAsyncStuff();
        return new myClass(async_result);
    }
}

Note: although in the examples above we use promises for the async builder they are not strictly speaking necessary. You can just as easily write a builder that accept a callback.

Note on calling functions inside static functions.

This has nothing whatsoever to do with async constructors but with what the keyword this actually mean (which may be a bit surprising to people coming from languages that do auto-resolution of method names, that is, languages that don't need the this keyword).

The this keyword refers to the instantiated object. Not the class. Therefore you cannot normally use this inside static functions since the static function is not bound to any object but is bound directly to the class.

That is to say, in the following code:

class A {
    static foo () {}
}

You cannot do:

var a = new A();
a.foo() // NOPE!!

instead you need to call it as:

A.foo();

Therefore, the following code would result in an error:

class A {
    static foo () {
        this.bar(); // you are calling this as static
                    // so bar is undefinned
    }
    bar () {}
}

To fix it you can make bar either a regular function or a static method:

function bar1 () {}

class A {
    static foo () {
        bar1();   // this is OK
        A.bar2(); // this is OK
    }

    static bar2 () {}
}

Constructor of a custom promise class is called twice (extending standard Promise)

First Update:

I first thought .catch( callback) after 'main' would return a new, pending promise of the extended Promise class, but this is incorrect - calling an async function returns a Promise promise.

Cutting the code down further, to only produce a pending promise:

class CancellablePromise extends Promise {

  constructor(executor) {

    console.log("CancellablePromise::constructor");

    super(executor);

  }

}

async function test() {

   await new CancellablePromise( ()=>null);

}

test();

Is It Bad Practice to Have a Constructor Function Return a Promise