Add objectMode option for objectMode streams

[indexzero]

This is necessary if you want to log the results of morgan to another system which performs serialization itself. e.g. interop with other logging libraries like winston using meta instead of writing a single log line:

var winston = require('winston')
var morgan = require('morgan')
var express = require('express')

var app = express()

app.use(morgan(function (tokens, req, res) {
  return {
    method: tokens.method(req, res),
    url: tokens.url(req, res),
    status: tokens.status(req, res)
  }
}, {
  stream: function (meta) { winston.info('Request served', meta) },
  objectMode: true
})

without objectMode: true in the above example winston would output:

Request served [object Object]\n

Given that objectMode streams are a very common use-case for streams this seems like an essential addition to morgan.

[dougwilson]

Cool. I'll fix up the issues with the PR when I merge :) !

[indexzero]

I checked the Travis build, was just some linting. My fingers implicitly type ; without me thinking about it even when I'm trying not to type them >_<

... should pass now.

[dougwilson]

I'm referring to other issues, not just the linting, which I saw you fixed before I made the comment. It's all good, I'll take care of it :)

[indexzero]

@dougwilson ok. Do what needs to be done. There are some time constraints on this one, so would appreciate if I didn't have to maintain a temporary fork 😸

[dougwilson]

Absolutely! Over these last few months I am under 80+ hour work weeks, so I'm very short on time, sadly. I mostly operate during transit on my phone, but I'm of course limited to what is possible to do by phone.

[indexzero]

Contributor ready, willing, and able if you feel like handing out commit bit and npm publish rights 🤓

[dougwilson]

This project is part of a project with governance; I do not have the ability to simply hand that out. I would absolutely be welcome to I boarding you into the Express.js project in line with the governance set out by the Express TC.

Even then, this PR needs a lot of work. I haven't made any comments, because I am very aware from our previous PR that trying to comment and work with you on changes to the PR can be taxing, and that you would prefer for me to simply fix the issues myself post merge. I am absolutely willing to do so, and is how I'm planning to proceed based on your feedback from your previous PR.

[dougwilson]

Generally, I think this is useful, and has come up sever times. One issue is that this module is fundamentally built to be a text line-based logger, and the PR is basically trying to hack in something else, rather than actually trying to restructure the underlying behavior to better cater to both cases. I have a lot of thoughts around this from the various issues that have been opened regarding this, and I think this PR can be used as the basis to push forward the rewrite I have in a stash on my machine to implement this as the final morgan 2.0. That's where my mind is on this PR, if that helps.

If you're willing to help, I can go through and push to a branch my WIP, and start discussing in here and we can work together towards this morgan 2.0 with the object support that you are looking for.

[indexzero]

👍 I'm always up for reading code if you can push up the WIP. Totally get the governance thing. Will read up on the charter and process.

[dougwilson]

Cool, I will prioritize getting that up and funneling my information here early next week, barring any issues on my end :) If you are looking for it, you can read the guide here: https://github.com/expressjs/express/blob/master/Contributing.md which outlines our process for how we accept PRs like this one, how people join, and more.

[dougwilson]

One thing you'll note in the guidelines is that it's not even possible for me to fix anything "post merge" as suggested; in order for me to do that, I would have to then open my own PR against this repo, probably including your changes and get it reviewed by another member in order to land, haha. That's why I wanted to work through getting the other PRs up to snuff so I could actually merge them, which is the exact same process that Node.js core uses. You'll always see all the "nit" comments on PRs, mainly because what gets merged has to be the exact contents of some PR, and it's easier just to do it on the original PR instead of go back and forth on different PRs (but you'll even see this happen in Node.js core as well :) ! ).

[indexzero]

Any progress on pushing that rewrite? It's been a month 😈

[antoniobusrod]

Hi @indexzero , do you want to fork to morgan-object NPM module?

[indexzero]

@antoniobusrod we quietly forked a while back to debra ... aka Dexter's sister on the show.

[johnantoni]

can this one go in?

[dougwilson]

No, it needs the changes noted above.

[johnantoni]

Ok, will see if I can resolve those, thanks.

[indexzero]

No, it needs the changes noted above.

Howdy @dougwilson could you clarify the changes? The actionable last comment from a year ago on this was:

"If you're willing to help, I can go through and push to a branch my WIP, and start discussing in here and we can work together towards this morgan 2.0 with the object support that you are looking for."

I assumed this was blocked until the rewrite you were working on was ready for review.

[dougwilson]

Hi @indexzero that's exactly correct. Is there confusion around that?

[johnantoni]

ok, thanks for clarifying