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

Provide a forked-version of repo that doesn't include aws-sdk #1128

Closed
baumannalexj opened this issue Feb 25, 2021 · 7 comments
Closed

Provide a forked-version of repo that doesn't include aws-sdk #1128

baumannalexj opened this issue Feb 25, 2021 · 7 comments

Comments

@baumannalexj
Copy link

Summary:

aws-sdk is already provided in the AWS Lambda: is there a way to choose not to include aws-sdk in this library? A forked version of the library where aws-sdk is a dev-dependency (dynamoose-no-aws)?

@fishcharlie
Copy link
Member

@baumannalexj I don't see this getting done anytime soon. PRs are always welcome tho. This would require changes to our build system to provide 2 versions. Most of that would probably be able to occur in https://github.com/dynamoose/dynamoose/blob/master/.github/workflows/publish.yml.

One other consideration is that v3 of Dynamoose will integrate AWS-SDK v3 as well. Which would probably break any functionality for this.

@baumannalexj
Copy link
Author

baumannalexj commented Feb 27, 2021

Thanks for addressing, @fishcharlie
To clarify, this build just needs "aws-sdk" module moved to dev-dependencies.

Right, it would need a second build. I do feel like aws convention is guiding development the wrong way though.

@fishcharlie
Copy link
Member

@baumannalexj

To clarify, this build just needs "aws-sdk" module moved to dev-dependencies.

Or removed all together. And this statement is also incorrect. The package name would also need to be modified. As far as I'm aware, there is no way to do that from the npm cli.

I do feel like aws convention is guiding development the wrong way though.

Broad statements like this are extremely unhelpful. Just saying something is wrong, without giving any detail, information, or solutions, is extremely unhelpful. How is it guiding development the wrong way? What changes do you think need to be made to address it? Really need more detail and information here.

@baumannalexj
Copy link
Author

baumannalexj commented Feb 27, 2021

@fishcharlie Correct, the package name would also need to be modified. Sorry for the back-and-forth: what I was clarifying was an idea for a solution (version of dynamoose exists where the aws-sdk was not included as a prod dependency). My apologies, this was a half-baked feature request, and a really we (my team) should have just stated the problem we were facing, instead of trying to provide a solution.

For now, we rm -rf node_modules/aws-sdk before zipping up our project and pushing to Lambda.

Regarding the broad statement: I agree, it was a vague throw away statement. It wasn't a remark that AWS has guided your team on making bad decisions, or anything of that nature. It was a comment on how we are learning to work with AWS Lambda, and didn't want to elaborate here, but can off this thread :].

@fishcharlie
Copy link
Member

fishcharlie commented Feb 27, 2021

@baumannalexj Sounds like we are on the same page. I'm all for this. Just not gonna be the highest priority for me right now. As I said, v3 will include changes to the AWS-SDK, so that might impact things as well. You are more than welcome to try to submit a PR for this if you'd like. I'd be happy to help answer any questions you have to make that happen. Thanks for the issue and hopefully we can get it implemented at some point!!

@fishcharlie
Copy link
Member

@baumannalexj dynamoose v3 alpha 1 has been released (#1040). This initial version only changes it to use AWS SDK v3 (more changes will be coming). The minified package size went from 2.8mb to 540kb (https://bundlephobia.com/result?p=dynamoose@3.0.0-alpha.1). I think this is a pretty impressive reduction in package size (which I'm assuming is your major goal here).

I have also opened an issue on the aws-sdk-js-v3 repo regarding wasteful package size: aws/aws-sdk-js-v3#2132. If that ever gets addressed by the AWS team, that will further reduce our package size.

I have also just checked, and according to https://docs.aws.amazon.com/lambda/latest/dg/lambda-nodejs.html, none of the Lambda Node runtimes support AWS SDK v3 yet.


All of that being said, here is what I am going to do/propose:

  1. I'm going to close this issue. I don't think any work can be done here since AWS SDK v3 isn't included on Lambda instances (if I'm wrong, please let me know).
  2. If you would like to have a fork for v2 that removes the AWS SDK, that is something that I'm gonna suggest you maintain on your own (or continue to use the rm tool to remove before zipping). I don't foresee Dynamoose providing that currently.
  3. Maybe consider giving my AWS issue a 👍, to show support for it? Or comment if you have additional ideas for how they can improve the size of the packages. Or if any of my ideas are bad, comment that too 😛.
  4. If you have further suggestions or things around this, please comment here and I'd be happy to reopen the issue.

Just for my reference in case this ever gets brought up again:

Might be worth considering using a NPM namespace @dynamoose and segmenting the project at some point. This would allow us to break things up further and allow users to only install what they need (ex. not all users need logging support, or source-map-support). Not quite sure how this would look yet. But might be a cleaner way to name packages and provide this functionality instead of a package such as dynamoose-no-aws.

@baumannalexj
Copy link
Author

That's would be an incredible reduction! Thank you @fishcharlie, that makes sense, feel free to close. I think we have our pipeline working with Artifactory+NPM, so we can maintain our own for now.

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

No branches or pull requests

2 participants