1

Could you please help to understand what can be wrong while using promises in callback on parent class and then trying to set properties on child class?

I'm using nodejs v8.2.1

Here is example of base class:

class CGLBase extends CAuthClient
{
    constructor(req, res)
    {   
        return new Promise(resolve => {
            super(req, res, (authClient) => {
                this.setAuthClient(authClient);
                resolve(this);
            });
        });
    }

    setAuthClient(authClient)
    {
       //setting authClient (this.auth will contain property)
    }
}

And here example of child class:

class СSheet extends CGLBase
{
    constructor(document, urlRequest, urlResponse) 
    {
        super(urlRequest, urlResponse);      

        this.document = document;
        this.someAnotherProp = "some property";
        //etc.. 
    }

    someFunc()
    {
       //using this.document and this.auth
    }
 }

After that I'm creating instance of СSheet and trying to set properties:

var document = { ... }; //here I create object

(async () => {
    var docSheet = await new СSheet (document, req, res);
    docSheet.someFunc();

    console.log(docSheet.auth);  //return correct property
    console.log(docSheet.document); //return undefined. why...
})();

So, I don't understand why property this.document wasn't set. I see only this.auth that was set in the async callback. All properties except this.auth are undefined.

I will be very grateful for advice or help.

Thank you in advance.

  • `this` is not available in ES6 constructors until *after* the call to `super`. When your callback arrow function is defined, `this` is still `undefined`. Also, I'm not sure why you would need to create an async constructor. It seems like this would be better implemented as an async static factory method. – sbking Aug 07 '17 at 17:09
  • 2
    Since when should constructors return promises? – PeterMader Aug 07 '17 at 17:09
  • 2
    Important read: [Is it bad practice to have a constructor function return a Promise?](https://stackoverflow.com/q/24398699/1048572) – Bergi Aug 07 '17 at 17:10
  • 2
    It's generally bad practice to have asynchronous constructors. See this answer: https://stackoverflow.com/questions/43431550/async-await-class-constructor – nicholas Aug 07 '17 at 17:21

1 Answers1

0

I don't understand why property this.document wasn't set. I see only this.auth that was set in the async callback.

Your CSheet constructor did set the document and someAnotherProp properties on the promise returned by super(). awaiting the new CSheet gives you the CAuthClient instance that the promise was resolved with, and which does not have the properties.

You could work around that by using

class СSheet extends CGLBase {
    constructor(document, urlRequest, urlResponse) {
        return super(urlRequest, urlResponse).then(instance => {
            instance.document = document;
            instance.someAnotherProp = "some property";
            // …
            return instance;
        });
    }
    …
}

but you really absolutely never should do that. This of course is the fault of CAuthClient taking an asynchronous callback. Fix that class and all its children to do the asynchronous creation of authClient in a static helper method, and only pass plain values into the constructor.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375