Inconsistent defaults and deprecation messages

[javierfernandes]

Hi guys !

This is probably a really tiny not-too-imporant bug, but well.. just found it and I think that it could help others.

Setting up express with morgan in this way

const app = express()
app.use(morgan)

Gives this deprecation warning (when trying to log a request, not at start up)

morgan deprecated morgan(options): use morgan("default", options) instead node_modules/express/lib/router/layer.js:95:5
morgan deprecated default format: use combined format node_modules/express/lib/router/layer.js:95:5

It is of course wrong, but I believe it was "the old way".
Anyway notice there that id proposes you to use default (use morgan("default", options))

But this is in turn deprecated, so if you change it like that you will get another deprecation message

const app = express()
app.use(morgan('default'))

You get

morgan deprecated default format: use combined format index.js:30:30

So, I believe that the first message should be updated to directly instruct you to use the "combined" option instead of the "default".

Thanks !
Regards !

[dougwilson]

Ah, good catch. Definitely makes sense! We'll get it updated, though if you want to take it up and make a pull request, feel welcome to do so 👍Just the change in message sounds spot on from your assessment without looking further at it.

[dougwilson]

So taking a look, I see what happened here and how you ended up with some weird messages. You didn't actually construct the middleware at all in your exanple, passing the factory as a middleware instead of an actual middleware:

const app = express()
app.use(morgan()) // note the extra parans

The weirdness I'm not sure can be fixed without actually breaking stuff. If you have an idea for how to do it, please feel free to pull request it. It is eluding me, at least.

[javierfernandes]

@dougwilson right ! Indeed I was using it wrongly.
So there are 2 different issues here.

1. using it (wrongly) directly as a middelware instead of calling it as a fn to get the middleware
2. this still happens if you call morgan({}), that is, with an object as fmt. It shows the outdated message

I've tackled 2) in #164

And proposed there a way to detect 1) and some doubts on how to handle it.

Please check that PR

Thanks !

Cheers

[dougwilson]

Thanks! The PR does the same thing I tried yesterday which doesn't work: the formats default and combined are different. You can't just suddenly change people's existing code to start using the combined format. That would be a semver major change, in which case all deprecations would be removed as part of any semver major update anyway.

[milosamec]

const morgan = require("morgan");
app.use(morgan("dev"));

Make sure you're not calling app.use(morgan) anywhere else in your code.