BackendStack21 / restana

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

Add new method: response.sendFile #88

Closed jesusvilla closed 4 years ago

jesusvilla commented 4 years ago

The treatment of files is too important, it would only be enough to do a .pipe and that's it, but there are more things that should be taken into consideration, such as adding the correct Content-Type header depending on the file format, as well as the Range header of the request , it is also important that there is a parameter to control the Content-Disposition and make it downloadable or not.

jkyberneees commented 4 years ago

Hi @jesusvilla, thanks for taking time to improve restana.

I agree on the necessity of having a sendFile abstraction to allow developers to easily deliver files from the server. However, I think in the context of restana (a REST oriented framework) this feature should be provided by an external middleware and not as a core functionality.

Related content: https://github.com/jkyberneees/restana-static

Regards

jesusvilla commented 4 years ago

I gree, but restana-static is more general, there may be the possibility at some point to use only sendFile.

jkyberneees commented 4 years ago

Hi @jesusvilla, I would still keep this feature out of restana core, as it will degrade performance for the other 90% of the current uses cases.

Regards

jesusvilla commented 4 years ago

How about writing something like extendResponse to add whatever method the develper prefers. Something like you did with response.send

jkyberneees commented 4 years ago

Hi @jesusvilla this is somehow my suggestion. Devs can always extend the response object class prototype in require('http').ServerResponse.prototype

Or just add it as an object method through a middleware:

app.use((req, res, next) => {
  res.sendFile = /* ... */
  return next()
})

However, the implementation of the actual sendFile method, should be out of restana core.

Regards