Node: New keyword returns cached module, is this behavior expected?

Created on 17 Sep 2016  Â·  6Comments  Â·  Source: nodejs/node

var Response = require('./utilities/res');

var res = new Response(false, "not working");
var res2 = new Response(true, "why");

Most helpful comment

@ArielSaldana @evanlucas @cjihrig Hello everybody, here is my opition :

Response.res is a global variable,so all instances share it 。I tried to modify respone.js , like this:

var Response = function (flag, message) {
this.res = {
success : null,
message : null,
payload : {}
}
this.res.success = flag;
this.res.message = message;
}

Response.prototype.addPayload = function(objname, obj) {
this.res.payload[objname] = obj;
}

Response.prototype.toJson = function() {
return this.res;
}

module.exports = Response;


and we can get the console results:
{ success: false, message: 'not working', payload: {} }
{ success: true, message: 'why', payload: {} }

@ArielSaldana Look forward to your views!

All 6 comments

can you give us a reproducible test case?

If I understand your question correctly, yes this is expected. Response appears to be a constructor function that is exported from './utilities/res'. The result of require() has nothing to do with the use of new.

Here is respone.js

var res = {
    success : null,
    message : null,
    payload : {}
}


var Response = function (flag, message) {
    res.success = flag;
    res.message = message;
    // res.payload = {};
}

Response.prototype.addPayload = function(objname, obj) {
    res.payload[objname] = obj;
}

Response.prototype.toJson = function() {
    return res;
}

// module.exports = Response;
exports.Response = Response;

and index.js

var Response = require('./utilities/res');

var res = new Response(false, "not working");
var res2 = new Response(true, "why");

console.log(res.toJson())    // returns { success: true, message: 'why', payload: {} }
console.log(res2.toJson()); // returns { success: true, message: 'why', payload: {} }

As you can see both return the same data, I expected it to create a new Response Object. Is there a way to have that functionality? As I understand it; the module is getting cached.

There is only one instance of res shared among all constructed Response objects. Closing as not a bug.

@ArielSaldana @evanlucas @cjihrig Hello everybody, here is my opition :

Response.res is a global variable,so all instances share it 。I tried to modify respone.js , like this:

var Response = function (flag, message) {
this.res = {
success : null,
message : null,
payload : {}
}
this.res.success = flag;
this.res.message = message;
}

Response.prototype.addPayload = function(objname, obj) {
this.res.payload[objname] = obj;
}

Response.prototype.toJson = function() {
return this.res;
}

module.exports = Response;


and we can get the console results:
{ success: false, message: 'not working', payload: {} }
{ success: true, message: 'why', payload: {} }

@ArielSaldana Look forward to your views!

Nice! I figured out the mistake after, I fixed the class and it looks like this now :

'use strict'

class Response {

    constructor(flag, message) {
        this.res = {
            success: null,
            message: null,
            payload: {}
        }
        this.res.success = flag;
        this.res.message = message;
    }

    toJson() {
        return this.res;
    }

    addPayload(objName, obj) {
        this.res.payload[objName] = obj;
    }
}

module.exports = Response;

Basically what you did, just using ES6 Classes.

Thank's for looking into it!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

vsemozhetbyt picture vsemozhetbyt  Â·  3Comments

fanjunzhi picture fanjunzhi  Â·  3Comments

willnwhite picture willnwhite  Â·  3Comments

mcollina picture mcollina  Â·  3Comments

filipesilvaa picture filipesilvaa  Â·  3Comments