Deprecate morgan.compile

[ryhinchey]

compile is a way to create a custom format function that can be passed to morgan. There's already a way to write a custom formatter function using morgan.format. Anything I can do with compile, I can do with a custom format function.

In addition, the compile function uses a method some would consider unsafe. I think if we can find a way to not dynamically create functions and call them, that's a win.

Given that compile returns a custom format function that I can then pass to morgan, why wouldn't I write a custom format function directly instead of using compile?

with compile:

var customFormatter = morgan.compile(':http-version :date[clf]')
var logger = morgan(customFormatter)

with custom formatter:

function customFormatter(tokens, req, res) {
  // assumes tokens are 
  return tokens['http-version'](req) + ' ' + tokens.date(req, res, 'clf')
}

var logger = morgan(customFormatter)

The compile function and token syntax does reduce the amount of code I have to write myself but I have to learn a new syntax and understanding how the compile function works could be confusing for newcomers to the library. Using only format functions keeps things easier to reason about and maintain in the future.

[dougwilson]

I think we need to get feedback from users before removing it, as this was a specific feature request by the module users (see #69 for instance).

[dougwilson]

why wouldn't I write a custom format function directly instead of using compile?

You wouldn't, of course. That is not the purpose of exporting the compile function.

[dougwilson]

In addition, the compile function uses a method some would consider unsafe. I think if we can find a way to not dynamically create functions and call them, that's a win.

Yes, I definitely agree on this. Of course I cannot recall anyone actually having an issue with this, and so I'm not sure if it makes sense to change this without any users actually asking for in. Conversely, it is there for performance concerns (which may or may not be founded any longer) since the typical arrangement of this module is to be involved in every request, making a performance here a top priority.

[dougwilson]

The compile function and token syntax does reduce the amount of code I have to write myself but I have to learn a new syntax and understanding how the compile function works could be confusing for newcomers to the library. Using only format functions keeps things easier to reason about and maintain in the future.

So this is why both are allowed: different people find different priorities. Personally many folks like myself keep the log format in configuration, which is not code and cannot contain functions. Having a token syntax is necessary for this use case.

[ryhinchey]

Conversely, it is there for performance concerns (which may or may not be founded any longer) since the typical arrangement of this module is to be involved in every request, making a performance here a top priority.

Definitely! Any proposals for changing this would have to be backed up by perf benchmarks compared to the existing implementation

[ryhinchey]

Personally many folks like myself keep the log format in configuration, which is not code and cannot contain functions. Having a token syntax is necessary for this use case.

great point! I didn't think about keeping these strings in a configuration file