Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export location of express built-in middlewares #78

Open
dougwilson opened this issue May 2, 2019 · 6 comments

Comments

Projects
None yet
3 participants
@dougwilson
Copy link
Member

commented May 2, 2019

So I'm working to land expressjs/express#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.

@LinusU

This comment has been minimized.

Copy link
Member

commented May 3, 2019

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!

@dougwilson

This comment has been minimized.

Copy link
Member Author

commented May 3, 2019

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.

@wesleytodd

This comment has been minimized.

Copy link
Member

commented May 3, 2019

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.

@dougwilson

This comment has been minimized.

Copy link
Member Author

commented May 3, 2019

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

@wesleytodd

This comment has been minimized.

Copy link
Member

commented May 3, 2019

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.

@dougwilson

This comment has been minimized.

Copy link
Member Author

commented May 3, 2019

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.

bors bot added a commit to jser/jser.github.io that referenced this issue May 28, 2019

Merge #617
617: 2019-05-28のJS: express 4.17.0、Firefox 67.0、TypeScriptのstrictNullCheckを有効化 r=azu a=azu

* [Release 4.17.0 · expressjs/express](https://github.com/expressjs/express/releases/tag/4.17.0)
  * [Export location of express built-in middlewares · Issue #78 · expressjs/discussions](expressjs/discussions#78)
    * [Expose bodyParser.raw on express by amitzur · Pull Request #3708 · expressjs/express](https://github.com/expressjs/express/pull/3708/files)
* [Strict null checking the Visual Studio Code codebase](https://code.visualstudio.com/blogs/2019/05/23/strict-null)
* [Firefox 67.0, See All New Features, Updates and Fixes](https://www.mozilla.org/en-US/firefox/67.0/releasenotes/)
  * [Backward-Compatibility FIDO U2F support shipping soon in Firefox | Mozilla Security Blog](https://blog.mozilla.org/security/2019/04/04/shipping-fido-u2f-api-support-in-firefox/)
  * [Firefox 67 for developers - Mozilla | MDN](https://developer.mozilla.org/ja/docs/Mozilla/Firefox/Releases/67)
    * [Firefox 67 サイト互換性情報 | Firefox サイト互換性情報](https://www.fxsitecompat.com/ja/versions/67/)
    * [Deprecated tools - Firefox Developer Tools | MDN](https://developer.mozilla.org/ja/docs/Tools/Deprecated_tools)
  * [コンテンツブロッキング | Firefox ヘルプ](https://support.mozilla.org/ja/kb/content-blocking)


Co-authored-by: azu <azuciao@gmail.com>
Co-authored-by: azu <azu@users.noreply.github.com>
Co-authored-by: probot-for-jser-info[bot] <probot-for-jser-info[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.