Why is "this" undefined in this class method?

Question

I've tried to search over what seems to be the entire internet, but I'm still vexed by a problem with a JS class I'm writing for a Micro Service (still in learning a bit).

So, I try to call a class method on an instantiated object, and according to my knowledge and my (faulty I presume) unit tests it should work.

Well, I'll start with the error I receive:

    GET /api/users 500 2.863 ms - 2649
TypeError: Cannot read property 'repository' of undefined
    at list (C:\Users\<user>\Documents\Programming\node\kaguwa-ngn\kaguwa-user-service\controllers\user-controller.js:20:9)
    at Layer.handle [as handle_request] (C:\Users\<user>\Documents\Programming\node\kaguwa-ngn\kaguwa-user-service\node_modules\express\lib\router\layer.js:95:5)
    at next (C:\Users\<user>\Documents\Programming\node\kaguwa-ngn\kaguwa-user-service\node_modules\express\lib\router\route.js:137:13)

(And a lot more).

The code calling code:

user-controller.js

'use strict';

var utils = require('./utils');

class UserController {

  constructor(repository) {
    this.repository = repository || {};
  }

  /**
   * 
   * Lists all users.
   * 
   * @param {object} req 
   * @param {object} res 
   */
  list(req, res) {

    this.repository.list(function (err, users) {
      if (err) return res.status(500).json(utils.createError(500));

      if (Object.keys(users).length !== 0) {
        res.json(users);
      } else {
        res.status(404).json(utils.createNotFound('user', true));
      }
    });
  }
// more code
}

module.exports = UserController

Caller of controller

user-api.js


'use strict';

var express = require('express');
var UserController = require('../controllers/user-controller');

var router = express.Router();

module.exports = function (options) {

  var userController = new UserController(options.repository);

  router.get('/users', userController.list);
  // Mode code

  return router;
};

I really jave no idea on why this undefined in UserController.

Any help would be greatly appriciated.

Solution 1

When you do this:

router.get('/users', userController.list);

what gets passed to your router is just a reference to the .list method. The userController instance gets lost. This is not unique to routers - this is a generic property of how things are passed in Javascript. To understand further, what you are essentially doing is this:

let list = userController.list; 
// at this point the list variable has no connection at all to userController
router.get('/users', list);

And, in Javascript's strict mode, when you call a regular function without any object reference such as calling list() above, then this will be undefined inside the function. That is what is happening in your example. To fix it, you need to make sure that your method is called with a proper object reference as in userController.list(...) so that the interpreter sets the this value appropriately.

There are multiple ways to solve this problem:

Make your own function wrapper

router.get('/users', function(req, res)  {
    userController.list(req, res);
});

This works in any version of Javascript.

---

Using .bind() to make a wrapper for you that calls it with the right object

router.get('/users', userController.list.bind(userController));

This works in ES5+ or with a .bind() polyfill.

---

Use an ES6 arrow function shortcut

router.get('/users', (...args) => userController.list(...args));

This works in ES6+

---

Personally, I prefer the .bind() implementation because I think it's just simpler and more declarative/clearer than any of the others and the ES6 "shortcut" isn't really shorter.

Solution 2

router.get() won't call your class the way you think it will. You are giving it a reference to a function which it will call in the context of the router.get meaning, it won't be in the context of your userController.

You can fix this by doing:

router.get('/users', function(){userController.list(...arguments)});

In other words, don't have express use userController's reference to list, have express use a closure that will have userController call list with the given arguments.

Comments

[Jose Carlos Ramos Carmenates]

Is the error when you call options.repository on the line ` var userController = new UserController(options.repository);` ?

[drinchev]

So can you show us the part of the code that calls require("user-api.js") ?

[KarlGdawg]

@drinchev the error was inside the code of user-repository, but the solutions provided by the other commenters. Thank you for your interest in the matter!

[alexmac]

Simplify this, use ES6 arrow functions: router.get('/users', () => userController.list()) or old bind method: router.get('/users', userController.list.bind(userController))

[zero298]

@alexmac I wanted to use an arrow function initially as well, but I think it would bind arguments incorrectly per MDN. Bind would work fine though.

[zzzzBov]

(...args) => userController.list(...args) for arrow function.

[jfriend00]

@zzzzBov - Corrected. Thx.

[Mark Adelsberger]

Note that class methods are called in strict mode, which is why you get this as undefined instead of "the wrong context"

[KarlGdawg]

@jfriend00 Thank you so much for your reply! I'm going to try that out, both solutions just to get a better hang of it. Didn't know that JS worked in that way. And this is some good knowledge to for future projects as well. Thank you! In earler projects I've used the `router.get('/users', function () {}) way... Don't know why I chose to do it like this this time around.

[KarlGdawg]

@zero298 Thank you for your reply! I've studied some JavaScript courses at university and a little in my spare time, but didn't know it passed it like that at all. Thank you!

[Mr5o1]

.bind is definitely best. consider: ``` class MyCls { constructor () { this.doSomething.bind(this)} doSomething () {...} } function myFn ({ foo }) { foo() } const myCls = new MyCls() myFn(myCls) ``` myFn destructures the arg passed in, so in that scope there's no reference to myCls, so you wouldn't be able to bind it there. Instead, you can bind it in the MyCls constructor. Admittedly, I haven't thought through this enough to decide whether it's a good idea, just saying bind seems most versatile.