Search code examples
javascriptnode.jsmemory-leakswinston

Does this closure cause a memory leak in Node.js or Is it a bad programming?

This is not a duplicate of will-this-closure-cause-a-memory-leak, because that is ios/swift and this is javascript/nodejs

For now I intended to use it for debugging only, and keep calling from anywhere like mocha/test, etc.. so it instantly log well formatted debug info/ (complex json objects) into test folder instead of server console.

Still curious to understand implication because for each call to such function will in turn call require('winston') which I am not sure is a good practice and second I want to ensure about memory leak.

exports.debugLog = (() => {
    const winston = require('winston');
    return winston.createLogger({
        level: 'info',
        format: winston.format.json(),
        defaultMeta: {service: 'debug-service'},
        transports: [new winston.transports.File({filename: 'debug.log'})]
    })
})()
Solution

  • I'm not sure what part of the code you think would cause a memory leak.

    It is not a problem to regularly call:

    const winston = require('winston');

    Modules are cached so after the module is loaded the first time, subsequent calls to require() that same module, just immediately returns the cached exports. It's fast and not a problem at all. In your specific case, I would probably write it like this:

    const winston = require('winston');

exports.debugLog = winston.createLogger({
        level: 'info',
        format: winston.format.json(),
        defaultMeta: {service: 'debug-service'},
        transports: [new winston.transports.File({filename: 'debug.log'})]
});

    Just because it's simpler to understand.

    It isn't clear at all why you were using the extra function wrapper. const winston is local only to this module so no real reason to try to hide it in an IIFE and IMO that wrapper just makes the code a lot more confusing to immediately grok what it's doing.

    Still curious to understand implication because for each call to such function will in turn call require('winston') which I am not sure is a good practice and second I want to ensure about memory leak.

    Each call to debugLog() will NOT be calling require('winston') again. debugLog is assigned the result of winston.createLogger() so that's what you're calling repeatedly, not the outer wrapper function you created. That wrapper function is only executed once at startup.