BackendStack21 / restana

Restana is a lightweight and fast Node.js framework for building RESTful APIs.
MIT License
467 stars 27 forks source link

Add more types in response.send #87

Closed jesusvilla closed 4 years ago

jesusvilla commented 4 years ago

Hi all. response.send should support the following variable types:

jkyberneees commented 4 years ago

Hi @jesusvilla, I agree with your suggestion. We should extend the current implementation and support those suggested types.

Would you like to contribute on this with a PR?

Regards

jesusvilla commented 4 years ago

Hi @jkyberneees Yeah, give me a few hours please.

jesusvilla commented 4 years ago

@jkyberneees for the Buffer types I have an observation:

Before:

service.get('/hello', (req, res) => {
  res.send(Buffer.from('Hello World!'))
})

Now:

service.get('/hello', (req, res) => {
  res.setHeader('content-type', 'text/plain')
  res.send(Buffer.from('Hello World!'))
})
jkyberneees commented 4 years ago

Hi @jesusvilla, I have reviewed and applied some minor changes to your PR on the extend-send-method branch. The major reason was performance improvements. New PR: https://github.com/jkyberneees/ana/pull/90

I kindly ask you to have a look, I would assume you also want to implement Promise support? Just let me know.

NOTE: Please consider to avoid performance degradation if you want to support promises.

Regards

jesusvilla commented 4 years ago

Hi @jkyberneees, I forgot to add Promise, sorry. btw, put some remarks on your commits, there are a lot of changes on my original code, I prefer you continue on Promise.

jkyberneees commented 4 years ago

Hi @jesusvilla I understand. Thank you very much for contributing, I will take it from here then.

Regards.

jkyberneees commented 4 years ago

Hi @jesusvilla, as described in https://github.com/jkyberneees/ana/pull/90, I have released the res.send method extensions with several fixes and support for Streams.

Promises are still not supported, I will followup on this in the future.

Regards.

edreesjalili commented 4 years ago

I took a shot at implementing the promises functionality. I don't think it performs too well at the moment, I want to see if I can optimize it in anyway before I make a PR to your master branch.

You can check it out here

Current results I am seeing.

[
  { name: 'statusCode', mean: 1.281798270451767e-9 },
  { name: 'undefined', mean: 1.2833191239290563e-9 },
  { name: 'string', mean: 4.857020931650364e-9 },
  { name: 'null', mean: 5.002937779672689e-9 },
  { name: 'buffer', mean: 2.746088882682896e-8 },
  { name: 'stream', mean: 2.9556736940863793e-8 },
  { name: 'string + headers', mean: 5.0168888821556044e-8 },
  { name: 'json', mean: 2.442347579351787e-7 },
  { name: 'error', mean: 2.6206075076260777e-7 },
  { name: 'json + headers', mean: 3.067224935837617e-7 },
  { name: 'promise', mean: 0.0000012263105912104794 }
]
jkyberneees commented 4 years ago

This @edreesjalili for supporting us here. With the merge of https://github.com/jkyberneees/ana/pull/93 I proceed to close this issue. Promise support will be available in v4.6.0.

Thank you!