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

Implement res.sendFile #17

Closed
talentlessguy opened this issue Jul 15, 2020 · 8 comments
Closed

Implement res.sendFile #17

talentlessguy opened this issue Jul 15, 2020 · 8 comments
Labels
core Affects `@tinyhttp/app` - the tinyhttp core enhancement New feature or request good first issue Good for newcomers

Comments

@talentlessguy
Copy link
Member

talentlessguy commented Jul 15, 2020

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

res.sendFile is commonly used in Express apps. tinyhttp must have one too.

Describe the solution you'd like

Implement res.sendFile based on existing res.send function

Additional context

Express docs reference: http://expressjs.com/en/5x/api.html#res.sendFile
Express implementation: https://github.com/expressjs/express/blob/ecd8a08c1c9c4d60b466b451fd1f5c3fe8a5fb23/lib/response.js#L403

@talentlessguy talentlessguy added enhancement New feature or request core Affects `@tinyhttp/app` - the tinyhttp core good first issue Good for newcomers labels Jul 15, 2020
@ahmad-reza619
Copy link
Contributor

Hi @talentlessguy i would like to take this on, is the shape of sendFile function has to be just like express?

@talentlessguy
Copy link
Member Author

@ahmad-reza619 yeah must work the same

code might differ

@talentlessguy
Copy link
Member Author

@ahmad-reza619 also, if possible, it shouldn't introduce any new dependencies

@ahmad-reza619
Copy link
Contributor

is this sendFile function will be able to send any kind of file? like maybe mp3 file?

@ahmad-reza619
Copy link
Contributor

From what i see from express test case, it looked like it only send file with text inside it

@talentlessguy
Copy link
Member Author

@ahmad-reza619 hmmm I think u're right but I by myself can't undertsand whether it sends a file with Content-Disposition header to download it, or just raw file

need to investiage more and re-check the implementation

@talentlessguy
Copy link
Member Author

implemented in 2bba94c, but not 1:1 to express

need more time to make it have the same API

@talentlessguy
Copy link
Member Author

I think it's implemented properly, if anyone has suggestions, feel free to write here and I will re-open the thread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Affects `@tinyhttp/app` - the tinyhttp core enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants