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

fix(bundlesize): move 'aws-lambda' as devDependency #331

Merged
merged 3 commits into from
Aug 30, 2022

Conversation

oscard0m
Copy link
Member

@oscard0m oscard0m commented Aug 30, 2022

Description

Move aws-lambda dependency from optionalDependency to devDependency

Context

This prevents users from installing aws-lambda since this dependency is installed by default.
Fixes #330

Previous context: #283

@wolfy1339 wolfy1339 changed the title fix(bundlesize): move 'aws-lambda' as deDependency fix(bundlesize): move 'aws-lambda' as devDependency Aug 30, 2022
@wolfy1339 wolfy1339 added the Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR label Aug 30, 2022
@oscard0m oscard0m force-pushed the move-aws-lambda-as-dev-dependency branch from dcaef4f to acb2aac Compare August 30, 2022 20:58
@oscard0m
Copy link
Member Author

@baoshan @wolfy1339 I would like you to take a look at this if you have time.
Thank you!

@wolfy1339
Copy link
Member

Can we remove it altogether instead? It's not used in the code, only the types package is

@oscard0m oscard0m force-pushed the move-aws-lambda-as-dev-dependency branch from acb2aac to 826ab52 Compare August 30, 2022 21:50
@oscard0m oscard0m force-pushed the move-aws-lambda-as-dev-dependency branch from 826ab52 to 1b299c4 Compare August 30, 2022 21:52
@wolfy1339
Copy link
Member

The types are a runtime dependency for typescript

@gr2m
Copy link
Contributor

gr2m commented Aug 30, 2022

Ugh. @wolfy1339 is right in that it is a production dependency when using typescript. It's really annoying, I really wish we had typeDependencies for use cases like this.

The only thing that I could think of would be to remove the middleware entirely from this repository, and release it in an external package, such as @octokit/middleware-aws-lambda-oauth or similar?

I really would like to remove aws-lambda from the production dependencies, it trickles down to @octokit/app and then octokit, and we are trying to make careful about external dependencies.

What do you think?

@oscard0m
Copy link
Member Author

The types are a runtime dependency for typescript

Good catch @wolfy1339. Always saving me 👼🏽

@oscard0m
Copy link
Member Author

oscard0m commented Aug 30, 2022

I really would like to remove aws-lambda from the production dependencies, it trickles down to @octokit/app and then octokit, and we are trying to make careful about external dependencies.

What do you think?

I completely agree. I created an issue.

In the meantime, is there an easy way to make our CI fail if we remove a required dependency for production as devDependency?

@wolfy1339
Copy link
Member

wolfy1339 commented Aug 30, 2022

I don't think there's an easy way to check, since we would still need dev dependencies for testing

@oscard0m oscard0m merged commit 0e0853e into master Aug 30, 2022
@oscard0m oscard0m deleted the move-aws-lambda-as-dev-dependency branch August 30, 2022 22:48
@github-actions
Copy link

🎉 This PR is included in version 4.0.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop pulling in aws-lambda which pulls in the huge aws-sdk
3 participants