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

Move @types/multer to dependencies #401

Merged
merged 1 commit into from Oct 21, 2020
Merged

Conversation

dprgarner
Copy link
Contributor

Types from @types/multer are being used in the exported framework/types.d.ts file. This breaks the TypeScript build of any project that uses this library but doesn't have @types/multer installed. Multer isn't a peerDependency of this library, so it shouldn't be a requirement for projects using express-openapi-validator to install it separately. It should be included as a dependency, and not a devDependency.

There could potentially be an issue with @types/express being used in framework/types.d.ts as well, but it's likely that any TypeScript project using this library will already have express types installed separately.

Copy link
Owner

@cdimascio cdimascio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dprgarner!

@cdimascio
Copy link
Owner

@all-contributors add @dprgarner for code

@allcontributors
Copy link
Contributor

@cdimascio

I've put up a pull request to add @dprgarner! 🎉

@cdimascio cdimascio merged commit 9f1c3d2 into cdimascio:master Oct 21, 2020
@pscanf
Copy link

pscanf commented Nov 8, 2020

@cdimascio I saw you ~reverted this PR soon after, were there any problems with moving @types/multer to dependencies?

(I noticed as updating to the latest version of express-openapi-validator once again gives me typescript errors regarding multer).

P.S. - since this is the first issue I open here, I'll take the chance to thank you for this amazing project! Great approach for building REST APIs and great DX of the library!

@cdimascio
Copy link
Owner

@dprgarner apologies, my mistake. This can go back to dependencies

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

Successfully merging this pull request may close these issues.

None yet

4 participants