Skip to content

feat(platform) multer support for fastify platform #2088

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

Closed
wants to merge 5 commits into from
Closed

feat(platform) multer support for fastify platform #2088

wants to merge 5 commits into from

Conversation

gperdomor
Copy link
Contributor

@gperdomor gperdomor commented Apr 26, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

File upload using Multer is only supported for express adapter
Issue Number: N/A

What is the new behavior?

Add fastify-multer support in @nestjs/platform-fastify.

Does this PR introduce a breaking change?

[] Yes
[x] No

Other information

Split multer code from platform-express to his own package
@gperdomor gperdomor changed the title Multer package WIP: Multer package Apr 26, 2019
@coveralls
Copy link

coveralls commented Apr 26, 2019

Pull Request Test Coverage Report for Build 2603

  • 66 of 68 (97.06%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 93.57%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/platform-fastify/multer/interceptors/file-fields.interceptor.ts 14 16 87.5%
Totals Coverage Status
Change from base Build 2599: 0.04%
Covered Lines: 3155
Relevant Lines: 3314

💛 - Coveralls

@coveralls
Copy link

Pull Request Test Coverage Report for Build 2578

  • 18 of 18 (100.0%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.07%) to 93.458%

Totals Coverage Status
Change from base Build 2574: -0.07%
Covered Lines: 3090
Relevant Lines: 3247

💛 - Coveralls

@gperdomor gperdomor changed the title WIP: Multer package Multer package Apr 26, 2019
@gperdomor
Copy link
Contributor Author

@kamilmysliwiec WDYT? 😄

@kamilmysliwiec
Copy link
Member

kamilmysliwiec commented Apr 27, 2019

Thanks for the contribution. A few things:

  • the original multer support (for express) is located in the @nestjs/platform-express package. Hence, we shouldn't create another lib but rather put it inside the @nestjs/platform-fastify instead of introducing breaking changes
  • fastify-multer is a too young library for now, we cannot recommend using it yet

@gperdomor gperdomor changed the title Multer package multer support for fastify platform Apr 27, 2019
@gperdomor
Copy link
Contributor Author

@kamilmysliwiec updated, Multer package was removed and the files related interceptors was copied to platform-fastify. Please consider a common @nestjs/multer package to avoid code duplication in Nest 7

@gperdomor
Copy link
Contributor Author

please let me know if you are ok with the new version to proceed to update the docs

@kamilmysliwiec
Copy link
Member

Thank you for your contribution @gperdomor. We have to postpone merging though because as I said:

fastify-multer is a too young library for now, we cannot recommend using it yet

Anyway, feel free to move forward with the docs. Once the library gets more mature, we'll surely make use of it.

@gperdomor
Copy link
Contributor Author

@kamilmysliwiec docs updated :D

@podzz
Copy link

podzz commented Jul 9, 2019

@kamilmysliwiec How long do you think this will take to merge this PR to master?

I am using @nestjs/platform-fastify and FileUpload in my project. In the meantime, is there a way to integrate this PR to @nestjs/platform-fastify dependency?

@kamilmysliwiec kamilmysliwiec changed the title multer support for fastify platform feat(platform) multer support for fastify platform Aug 24, 2019
@zuohuadong
Copy link

@kamilmysliwiec @gperdomor
Any progress?

@ap-gorkun
Copy link

ap-gorkun commented May 15, 2020

@kamilmysliwiec @gperdomor
fastify-multer has aged one year since the date of this PR. WDYT now? 😄

@mubasshir
Copy link

@kamilmysliwiec any update on it? we are missing out on performance due to this small part

@gperdomor
Copy link
Contributor Author

Thanks to everyone for for the interest in this PR, I will close this in favor of #4921 which basically is an updated version of this one

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

Successfully merging this pull request may close these issues.

7 participants