Closed tusharf5 closed 5 months ago
This is a very good idea, please contribute!
Generally we do not want to include vendor-specific dependencies in the client by default.
@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 - https://github.com/evanw/esbuild/issues/1435
or maybe we remove v2 completely and just use @aws-sdk/credential-provider-node and mark it as a required peer dependency?
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).
I'll look into this and see what all options do we have.
Thank you! Just make PRs, we'll try to work through the best approach.
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