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

Sending Files #5263

Closed
jmcdo29 opened this issue Aug 12, 2020 · 3 comments · Fixed by #5278
Closed

Sending Files #5263

jmcdo29 opened this issue Aug 12, 2020 · 3 comments · Fixed by #5278

Comments

@jmcdo29
Copy link
Member

jmcdo29 commented Aug 12, 2020

Feature Request

I didn't see any old/closed issues referring to supporting sending files other than support requests that were directed towards Discord.

I'd like to see Nest support sending files out of the box, by making a check in the reply method of the ExpressAdapter and FastifyAdapter and using the proper method accordingly (it looked like Fastify can actually handle this under its hood, so we'd just need to support it with Express.

Is your feature request related to a problem? Please describe.

Not really a problem, but sending files is somewhat common, and having to use @Res() to do it is a major buzzkill for me in the Nest world. Losing the use of post-request interceptor functionality is really annoying, especially when it comes to logging.

Describe the solution you'd like

I'd like to see Nest support the returning of a Buffer or a stream and calling the appropriate method from Express stream.pipe(res) or Fastify reply.send(stream) to send the file back successfully.

Teachability, Documentation, Adoption, Migration Strategy

Add to the docs that sending files is supported. Shouldn't need much more teachability, and it shouldn't cause any breaking changes.

What is the motivation / use case for changing the behavior?

Sending files 📁 because there's definitely a use case for that.

I'd be more than happy to get to work on this and add it in, but I'd love to hear some feedback first.

@jmcdo29 jmcdo29 added needs triage This issue has not been looked into type: enhancement 🐺 labels Aug 12, 2020
@kamilmysliwiec
Copy link
Member

Would you like to create a PR for this? :)

@kamilmysliwiec kamilmysliwiec removed the needs triage This issue has not been looked into label Aug 13, 2020
@jmcdo29
Copy link
Member Author

jmcdo29 commented Aug 13, 2020

You bet! I'll see what I can have by the end of the weekend

jmcdo29 added a commit to jmcdo29/nest that referenced this issue Aug 15, 2020
Fastify has the built in capbility to send files without
any extra code. Express can send files via `stream.pipe(res)`
if the file is a stream, and as such, the
`Content-Type` header is set to `application/octet-stream`
in the ExpressAdapter if a stream or Buffer is detected.

fix nestjs#5263
@kamilmysliwiec
Copy link
Member

Let's track this in a single place, here #5278

jmcdo29 added a commit to jmcdo29/nest that referenced this issue Dec 10, 2020
Fastify has the built in capbility to send files without
any extra code. Express can send files via `stream.pipe(res)`
if the file is a stream, and as such, the
`Content-Type` header is set to `application/octet-stream`
in the ExpressAdapter if a stream or Buffer is detected.

fix nestjs#5263
jmcdo29 added a commit to jmcdo29/nest that referenced this issue Jan 28, 2021
Fastify has the built in capbility to send files without
any extra code. Express can send files via `stream.pipe(res)`
if the file is a stream, and as such, the
`Content-Type` header is set to `application/octet-stream`
in the ExpressAdapter if a stream or Buffer is detected.

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

Successfully merging a pull request may close this issue.

2 participants