fix: respect original req.body if it is still a buffer #22
Conversation
| const requestBody = | ||
| typeof req.body === 'string' ? req.body : JSON.stringify(req.body) | ||
| req.body instanceof Uint8Array |
There was a problem hiding this comment.
This looks great!
Please, could you also add a test for this behavior? Take a look at the existing tests for inspiration.
This scenario would come down to:
- Prepare the setup, have no
express.json()middleware. - Perform a request.
- Assert the correct request body.
- Prepare the setup, have the
express.json()middleware. - Perform a request.
- Assert that the body is still correct.
We can put these two in a single test file to reflect on this behavior.
There was a problem hiding this comment.
yeah, sure thing!
There was a problem hiding this comment.
After setting up this test, I discovered that the root cause of my issue was that I was using express.raw() instead of express.json() (as opposed to no middleware, as I originally assumed) as a workaround for #17.
In the end, some body parsing middleware is necessary, so maybe this change is unnecessary. However, I still think this change is worthwhile, as using express.raw() is technically more performant since it doesn't have to parse/unparse.
I also had to make the test setup a bit more complicated since the open-draft test-server automatically applies bodyParser.json() middleware and there is no way to disable it.
robbtraister
force-pushed
the
bufix/req.body-buffer
branch
from
d7dc876
to
50e0c3c
Compare
|
Would be great if this gets merged, running against this issue too 👍 |
| }) | ||
|
|
||
| /** | ||
| * The test server created by @open-draft/test-server automatically |
There was a problem hiding this comment.
This is peculiar. I need to look into disabling/customizing the default middleware.
This PR coerces the MockedRequest body to be a buffer from a string (or stringified object). This works well if
express.json()middleware has been mounted because we need to un-parse the work the middleware performed.However, that in effect makes the
express.json()middleware required, because otherwise we are undoing work that was never done and we end up stringifying the originalUint8Arraybuffer, which does not work as expected.This change preserves the original body buffer if no
express.json()(or similar) middleware has been mounted, allowing the subsequentreq.json()call to work as expected, regardless.