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

Expose bodyParser.raw on express #3708

Merged
merged 2 commits into from
May 9, 2019
Merged

Expose bodyParser.raw on express #3708

merged 2 commits into from
May 9, 2019

Conversation

amitzur
Copy link
Contributor

@amitzur amitzur commented Aug 8, 2018

It's currently not possible to use the bodyParser.raw middleware through express. To have it one needs to use bodyParser directly. But alas, body-parser is not exposed through exports. So it's necessary to add it as a dependency to your project, which is clearly not desired as it might lead to multiple versions of body-parser (see #3706).

This PR just adds the raw middleware to the exports object, same as json and urlencoded.

@wesleytodd wesleytodd self-requested a review August 8, 2018 16:41
@dougwilson
Copy link
Contributor

Can someone stage a PR for the documentarion of this against the expressjs.com repo? That would be a huge help since there is a lot to document here. I can write this up myself as well, though that just means waiting on me :)

Copy link
Contributor

@dougwilson dougwilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a test somewhere that checks just that it works. This is usually done as an acceptance test.

@bradbforbes
Copy link

From what I understand about NPM, it's designed to handle these types of conflicts seamlessly because each dependency can have its own version of a dependency in its tree. If your project is using body-parser itself, it may make more sense to use it as its own dependency instead of depending on Express to expose its methods in the way you need it to, now and in the future. This is probably worth a small increase in the size of your code.

@dougwilson dougwilson added this to the 4.17 milestone Sep 24, 2018
@dougwilson dougwilson mentioned this pull request Oct 27, 2018
23 tasks
@dougwilson dougwilson self-assigned this May 1, 2019
@dougwilson
Copy link
Contributor

@LinusU @wesleytodd I'm working to land this 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?

This side discussion is not a blocker to landing this in 4.17.

I created expressjs/discussions#78 to discuss further.

@dougwilson dougwilson changed the base branch from master to 4.17 May 2, 2019 22:52
@dougwilson dougwilson force-pushed the raw branch 2 times, most recently from c7aa250 to 87ee733 Compare May 8, 2019 04:29
@dougwilson dougwilson merged commit 11192bd into expressjs:4.17 May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants