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

docs: consume req.body in the form of key-value pairs #233

Merged
merged 1 commit into from
May 15, 2021
Merged

docs: consume req.body in the form of key-value pairs #233

merged 1 commit into from
May 15, 2021

Conversation

max-hk
Copy link
Contributor

@max-hk max-hk commented May 14, 2021

Checklist

The pull request add documentation on how to consume req.body in key-value pairs.

This may help new comers from Express.js

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina requested a review from StarpTech May 14, 2021 15:43
@max-hk
Copy link
Contributor Author

max-hk commented May 14, 2021

Hi. I found that most code snippets in readme do not contain trailing semicolon.

Should I remove semicolon from my pull request, or keep it as is?

@mcollina
Copy link
Member

thanks, that'd be best!

README.md Outdated Show resolved Hide resolved
@max-hk
Copy link
Contributor Author

max-hk commented May 14, 2021

After testing in my machine, I found that req.body.foo.value returns the value itself instead of a promise.

Maybe we should remove the await keyword?

fastify.post('/upload/files', async function (req, reply) {
  const uploadValue = await req.body.upload.toBuffer()  // access files
-  const fooValue = await req.body.foo.value           // other fields
+  const fooValue = req.body.foo.value                 // other fields
  const body = Object.fromEntries(
    Object.keys(req.body).map((key) => [key, req.body[key].value])
  ) // Request body in key-value pairs, like req.body in Express (Node 12+)
})

@mcollina
Copy link
Member

sure thing

@mcollina
Copy link
Member

Can you add a unit test for this? So it's sure we won't regress.

@max-hk
Copy link
Contributor Author

max-hk commented May 15, 2021

Can you add a unit test for this? So it's sure we won't regress.

Is the following line enough? Should I write another test for that?

t.equal(req.body.hello.value, 'world')

@mcollina mcollina merged commit d615973 into fastify:master May 15, 2021
@mcollina
Copy link
Member

actually it is! Landed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants