13

I am making an app using Node.js, Express.js and MongoDB. I am using a MVC pattern and also have separate file for routes. I am trying me make a Controller class, in which a method calls another method declared within it. But I cannot seem to be able to do this. I get "Cannot read property '' of undefined".

index.js file

let express = require('express');
let app = express();

let productController = require('../controllers/ProductController');

app.post('/product', productController.create);

http.createServer(app).listen('3000');

ProductController.js file

class ProductController {
  constructor(){}

  create(){
   console.log('Checking if the following logs:');
   this.callme();
  }

 callme(){
  console.log('yes');
 }
}
module.exports = new ProductController();

When I run this I get following error message:

Cannot read property 'callme' of undefined

I have ran this code by itself with little modification as following and it works.

class ProductController {
  constructor(){}
  create(){
    console.log('Checking if the following logs:');
    this.callme();
  }

  callme(){
    console.log('yes');
  }
}
let product = new ProductController();
product.create();

Why does one work and not the other? HELP!

Kucl Stha
  • 565
  • 1
  • 6
  • 20
  • 2
    You should [never export a class instance](http://stackoverflow.com/a/39079929/1048572). Either export the class itself, or use an object only. – Bergi Sep 21 '16 at 17:16
  • You should define the methods inside your class using the property initializer syntax (`callme = () => {...}` instead of like this `callme() {...}`). https://github.com/facebook/flow/issues/5874#issuecomment-369922816 – Derek Soike Jan 10 '19 at 02:02

2 Answers2

6

Your method is being rebound to the Layer class within express, losing its original context. The way that express handles routes is by wrapping each one in a Layer class, which assigns the route callback to itself:

this.handle = fn;

That is where your problems arise, this assignment automatically rebinds the function context to Layer. Here is a simple example demonstrating the problem:

function Example() { 
   this.message = "I have my own scope"; 
} 
Example.prototype.logThis = function() { 
   console.log(this); 
}

function ReassignedScope(logThisFn) { 
   this.message = "This is my scope now";
   // simulation of what is happening within Express's Layer
   this.logThis = logThisFn; 
}

let example = new Example()
let scopeProblem = new ReassignedScope(example.logThis);

scopeProblem.logThis(); // This is my scope now

Others have already pointed out the solution, which is to explicitly bind your method to the ProductController instance:

app.post('/product', productController.create.bind(productController));
Rob M.
  • 35,491
  • 6
  • 51
  • 50
5

When you pass create method as method it is probably called in different context (this) as you expect. You can bind it:

app.post('/product', productController.create.bind(productController));

There are many other ways how to ensure this refers to correct object.

E.g. wrap it with function (either arrow or classical):

app.post('/product', (...args) => productController.create(...args));

Or bind methods in constructor:

constructor() {
    this.create = this.create.bind(this);
}
madox2
  • 49,493
  • 17
  • 99
  • 99