Conversation
|
Did you two decide not to move forward with this? |
|
@cecilia-donnelly you know what, I didn't -- I just forgot I had started it after pulling into another priority at the time. I'll finish this up shortly. What's missing is the replacement of the middleware (I took out body parser but didn't add back |
slifty
force-pushed
the
noissue-remove-body-parser
branch
2 times, most recently
from
073d915 to
ad1e2fc
Compare
Express now natively supports `.json` so we don't need to use the body-parser's version of the middleware. Issue #284 Removing body-parser
liam-lloyd
force-pushed
the
noissue-remove-body-parser
branch
2 times, most recently
from
e0d461e to
0e8245f
Compare
Before switching to the express.json() middleware, empty requests bodies
were represented by empty objects, meaning middleware that added things
to the request body was always able to do so. Now, however, empty
request bodies are undefined, meaning such middleware fails. This commit
adds an extra piece of middleware to run after express.json() and set
request.body to {} if it is undefined, allowing the rest of the
middleware to run successfully. Note that this is a stopgap, we should
eventually update all the middleware not to modify the request body.
liam-lloyd
force-pushed
the
noissue-remove-body-parser
branch
from
0e8245f to
621b13e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #283 +/- ##
=======================================
Coverage 98.21% 98.22%
=======================================
Files 101 101
Lines 2414 2417 +3
Branches 401 400 -1
=======================================
+ Hits 2371 2374 +3
Misses 39 39
Partials 4 4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I'd like to be rid of body-parser, particularly because of the bizarre |
|
@liam-lloyd oh brilliant patch thank you! I'm going to review to approve your portion ;) |
|
Oh, I can't review my own PR (this makes sense) Which is to say: @liam-lloyd if you wanna review + approve I'll merge! |
|
🎉 🎉 🎉 |
3 participants
This PR removes an unnecessary dependency (Express provides a native version of
.json()middleware now)Resolves #284