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

dynamic import bundles entire aws-sdk #757

Closed
tusharf5 opened this issue Apr 16, 2024 · 6 comments · Fixed by #758
Closed

dynamic import bundles entire aws-sdk #757

tusharf5 opened this issue Apr 16, 2024 · 6 comments · Fixed by #758
Labels
🧗 enhancement New feature or request

Comments

@tusharf5
Copy link
Contributor

Is your feature request related to a problem?

When bundling opensearch-js with webpack or esbuild, the entire aws-sdk gets included in the bundle because of the import here https://github.com/opensearch-project/opensearch-js/blob/main/lib/aws/AwsSigv4Signer.js#L30C5-L30C43

This adds an additional 11MB to the final bundle size.

What solution would you like?

The code can be refactored to make it a static import that can be treeshook, maybe separate files for aws sdk v2 & aws sdk v3.

What alternatives have you considered?

We've marked aws-sdk as external to keep it from bundling but that's risky as some other library might need aws-sdk

@dblock
Copy link
Member

dblock commented Apr 16, 2024

This is a very good idea, please contribute!

Generally we do not want to include vendor-specific dependencies in the client by default.

@dblock dblock added 🧗 enhancement New feature or request and removed untriaged labels Apr 16, 2024
@tusharf5
Copy link
Contributor Author

tusharf5 commented Apr 16, 2024

@dblock Are you okay with the approach? Separate imports for v2 and v3

So a client using aws-sdk v3 could do

const { AwsSigv4Signer } = require('@opensearch-project/opensearch/aws-v3'); // this would not use aws-sdk v2

and a client using aws-sdk v2 would use

const { AwsSigv4Signer } = require('@opensearch-project/opensearch/aws-v2'); // this would only use aws-sdk v2

and add @aws-sdk/credential-provider-node, aws-sdk v2 as optional peer dependencies?

We would still keep

const { AwsSigv4Signer } = require('@opensearch-project/opensearch/aws'); // uses both

for backwards compatibility.

Seems like just making them as static imports in the same file will not fix the bundling issue - evanw/esbuild#1435

@tusharf5
Copy link
Contributor Author

or maybe we remove v2 completely and just use @aws-sdk/credential-provider-node and mark it as a required peer dependency?

@dblock
Copy link
Member

dblock commented Apr 16, 2024

I am not an expert on this so I am not sure what the best approach is. I definitely think we must support both v2 and v3 though, and preserve backwards compatibility unless we want to major-increment the version (which we should avoid if it's possible).

@tusharf5
Copy link
Contributor Author

I'll look into this and see what all options do we have.

@dblock
Copy link
Member

dblock commented Apr 16, 2024

Thank you! Just make PRs, we'll try to work through the best approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧗 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants