discussions icon indicating copy to clipboard operation
discussions copied to clipboard

Export location of express built-in middlewares

Open dougwilson opened this issue 6 years ago • 6 comments

So I'm working to land https://github.com/expressjs/express/pull/3708 in the 4.17 branch now and I thought that express.raw() seems like a strange name, which made me think that handing the middlewares right off the express export itself is maybe weird?

I was thinking maybe we could put them under like .middleware or, if not, corral the parsers under .bodyParser maybe.

The following are the current middlewares:

  • express.json
  • express.urlencoded
  • express.static
  • express.query

The following are the 4.17 proposed middlewares:

  • express.raw
  • express.text

So I was thinking we maybe could do

  1. express.middleware.static etc.
  2. express.static and express.bodyParser.json, etc.
  3. keep as-is

Maybe something else, even.

@LinusU @wesleytodd do you have any thoughts? This wouldn't block express.raw unless there was a general agreement like express.middleware.raw or something and the new ones could be landed under the new name up front and the rest moved instead of moving them all later, idk.

dougwilson avatar May 02 '19 20:05 dougwilson

Yeah express.raw seems a bit strange, almost express.text as well...

I think that I really like option 2, potentially named bodyParsers though 🤔 or maybe not 😆

Naming things is always hard!

LinusU avatar May 03 '19 13:05 LinusU

Naming things is always hard!

Indeed! That's why I'm not going to prevent landing those (perhaps weird) names for the time being so folks can use them until there is a decision :) We can always rename them with an alias back to the old name.

dougwilson avatar May 03 '19 14:05 dougwilson

I am on the fence. I do agree it feels weird. But I am not sure it improves the situation to move it. This would cause churn, and unless there was some strong benefit I am not sure the churn is worth it. I could agree with any of the three options if someone else had a stronger opinion.

wesleytodd avatar May 03 '19 17:05 wesleytodd

Yea, I definitely agree on the churn point. That's why I realized I should open this with at least some time before express.raw and express.text became part of the API, haha

dougwilson avatar May 03 '19 17:05 dougwilson

I edited not thinking people would see this so quickly, so moving it to a separate comment.

I know this is not really a helpful response in making the decision. So I guess if I was forced to pick one I would also go with option 2 but with the plural like @LinusU mentioned.

wesleytodd avatar May 03 '19 17:05 wesleytodd

One nice thing, thinking about the plural form, is that express.bodyParser() was a middleware in 3.x. It was just json + urlencoded. But I don't really want to bring that pattern back, and so using express.bodyParsers.raw(), for example, would still keep express.bodyParser the throw accessor like the other removed middlewares.

So it sounds like we're leaning (I used this uncertain term on purpose) towards the following set up, right?

  • express.bodyParsers.json
  • express.bodyParsers.raw
  • express.bodyParsers.text
  • express.bodyParsers.urlencoded
  • express.static
  • express.query

The express.query will be gone in 5.0 as that is just a direct req.query sugar instead of a middleware there.

The express.json and express.urlencoded is still TBD. Above I was just laying out I suspect the "desired" state, not really saying what would happen with these two at this time.

dougwilson avatar May 03 '19 17:05 dougwilson