2

I am developing a NodeJS package that contains a Thenable class (see Thenable objects) that is supposed to return this (the initialized object itself), but instead it is returning a new, uninitialized object and it is stuck in a neverending loop. Below is the code:

node_modules/<my package>/index.js

const axios = require('axios');
const config = require('./config.json');

class MyAPI {

  #token = "";

  constructor(username, password) {
    this.#token = token;
  }

  then(resolve, reject) {
    const options = {
      url: "/api/authenticate",
      baseURL: "https://myapi.com/",
      method: 'post',
      maxRedirects: 0,
      data: {
        token: this.#token
      },
      transformRequest: this.#transformRequest('urlencoded'),
      transformResponse: this.#transformResponse
    };
    axios.request(options).then((res) => {
      if (res.data.authenticated === true) {
        console.log("Authenticated!");
        resolve(this); // <-- should return the initialized object, but instead returns a new uninitialized object
      } else {
        reject(new Error('Invalid credentials'));
      }
    });
  }

  #transformRequest(type) {
    if (type === 'urlencoded') {
      return function(data) {
        return (new URLSearchParams(data)).toString();
      }
    } else if (type === 'json') {
      return function(data) {
        return JSON.stringify(data);
      }
    }
  }

  #transformResponse(data) {
    return JSON.parse(data);
  }

  getSomething() {
    // Gets something from remote API
    return response;
  }

}

module.exports.MyAPI = MyAPI;

index.js

const { MyAPI } = require('<my package>');
(async function(){
  try {
    const app = await new MyAPI(config.auth.token);
    console.log("App initialized!"); // Code never reaches this command
    console.log(app.getSomething());
  } catch (e) {...} // Handle the error
})()

The log gets filled up with "Authenticated!" and the code never makes it to console.log("App initialized!"). I've seen this answer but it does not help me because I know this is referring correctly to the object.

Replacing resolve(this) with resolve() stops this, but await new MyAPI() resolves to undefined and I cannot later run app.getSomething().

In a non-Thenable class (e.g. new MyClass()), it resolves to the object itself, so further operations can be done on it, so why can't await new MyAPI() also resolve to the object itself like a constructor should?

Maytha8
  • 846
  • 1
  • 8
  • 26
  • 3
    "*a thenable that is supposed to [resolve with] `this`*" - not sure why you would want that. Is your actual goal to make the `const app = await new MyAPI(config.auth.token);` syntax work? If so, I suggest to have a look at [Is it bad practice to have a constructor function return a Promise?](https://stackoverflow.com/q/24398699/1048572) – Bergi Jan 11 '22 at 08:35
  • @Bergi `new MyAPI()` resolves to the object itself, so further operations can be done on it, so why can't `await new MyAPI()` resolve to the object itself like a constructor should? – Maytha8 Jan 11 '22 at 09:11
  • @Maytha8 `new MyAPI()` returns a thenable. When you `await` it, it's calling the `.then()`. Which returns the same instance. Which is a thenable. When you `await` it, it's calling the `.then()`. Which returns the same instance. Which is a thenable. When you `await` it, it's calling the `.then()`. Which returns the same instance. Which is a thenable. Can you see where the problem is? – VLAZ Jan 11 '22 at 10:40
  • I meant that if `MyAPI` wasn't a thenable. I'll rephrase. `class MyAPI { constructor(token) {this.#token = token;} doSomething() {...}}`. `const app = new MyAPI(token)` resolves to a class. `app.doSomething()` does something and does not throw an error. Now let's go back to the thenable. `const app = await new MyAPI(token);` keeps looping. You can't then do `app.getSomething()`. If I changed `resolve(this)` to `resolve()`, then `app === undefined //true` and you still can't do `app.getSomething()`. Can you see where the problem is? – Maytha8 Jan 11 '22 at 11:14
  • I already said what you said in my question, and I can see the problem. That's why this question exists in the first place @VLAZ. The title says 'How do I **properly return 'this'** in Thenable objects?' – Maytha8 Jan 11 '22 at 11:15
  • Again, the problem is that you resolve the `.then()` with a thenable. With an `await` that thenable also has to be resolved until a plain *non-thenable* value. But since it always resolves to a thenable, it has to be resolved infinitely. Your question here is asking the wrong thing, because the proper thing isn't to try and jury-rig a `.then()` to do some instantiation logic and return the same instance. Hence why Bergi's answer doesn't do that. And seems you accepted it - I don't think you should go back to asking about writing weird unconventional code that requires hacks to even work. – VLAZ Jan 11 '22 at 11:40

1 Answers1

2

Just don't do that. Instead of making your objects thenable, give them an init method that simply returns a promise. Then use them as

const app = new MyAPI(config.auth.token);
await app.init();
console.log("App initialized!");
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • For sure a better practice. Why not a factory though? Seems like that would be less error prone than 2 phase construction. – Aluan Haddad Jan 11 '22 at 08:46
  • 2
    @AluanHaddad I do definitely recommend a factory (e.g. a static method on the class), but this was the smallest change to their current code and I'm not entirely certain what their actual goal was. With an `init` method, you can potentially initialise the same instance twice (e.g. if the first authentication attempt failed). – Bergi Jan 11 '22 at 08:48
  • Is it better to return a `new Promise`, or to make the function `async` and `await` the axios request? – Maytha8 Jan 11 '22 at 09:17
  • 2
    @Maytha8 Don't use `new Promise`, `axios` already returns one. You can either `await` it or `return axios(…).then(…);`. – Bergi Jan 11 '22 at 09:20
  • I moved the authentication from the constructor to a new method, `static authenticate(token)`. – Maytha8 Jan 11 '22 at 09:22