Avoid importing full root @pulumi/aws module
#2569
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Every folder inside the AWS Node.js SDK that has a mixin was importing "../utils", which imports "./awsMixins" which itself was forcing import of "." which then goes back and tries to force import of everything. There is no clear reason why forcing this root module import was ever necesary. Nothing that imports utils.ts depends on the side effects caused by "awsMixins", and the primary effect of adding the `sdk` getter onto the root module is only relevant if the end user loads that root module itself so they can access this property. Also expands some existing test coverage to test this and use of `aws.sdk`, which is the related feature that could in principle be impacted by removing this forced side effect Fixes #772.
Does the PR have any schema changes?Looking good! No breaking changes found. |
1 similar comment
Does the PR have any schema changes?Looking good! No breaking changes found. |
rquitales
approved these changes
Jun 23, 2023
|
Just ran into this myself, thanks for fixing this @lukehoban! Looks like this hasn't made its way into a release yet, I'll use a local patch in the mean time. |
iwahbe
pushed a commit
that referenced
this pull request
Jul 12, 2023
Every folder inside the AWS Node.js SDK that has a mixin was importing "../utils", which imports "./awsMixins" which itself was forcing import of "." which then goes back and tries to force import of everything. There is no clear reason why forcing this root module import was ever necesary. Nothing that imports utils.ts depends on the side effects caused by "awsMixins", and the primary effect of adding the `sdk` getter onto the root module is only relevant if the end user loads that root module itself so they can access this property. Also expands some existing test coverage to test this and use of `aws.sdk`, which is the related feature that could in principle be impacted by removing this forced side effect Fixes #772.
iwahbe
pushed a commit
that referenced
this pull request
Jul 13, 2023
* Avoid importing full root `@pulumi/aws` module (#2569) Every folder inside the AWS Node.js SDK that has a mixin was importing "../utils", which imports "./awsMixins" which itself was forcing import of "." which then goes back and tries to force import of everything. There is no clear reason why forcing this root module import was ever necesary. Nothing that imports utils.ts depends on the side effects caused by "awsMixins", and the primary effect of adding the `sdk` getter onto the root module is only relevant if the end user loads that root module itself so they can access this property. Also expands some existing test coverage to test this and use of `aws.sdk`, which is the related feature that could in principle be impacted by removing this forced side effect Fixes #772. * Remove aws.sdk property The `aws.sdk` proeprty was introduced several years ago to offer a convenient way for callbacks using function serialization to get ahold of the AWS SDK which is available within the Lambda runtime. However, the AWS SDK v2 that was exposed here is being deprecated, and has been emitted a deprecation warning for some time for anyone using this via Pulumi. The AWS SDK v3 doesn't expose a monolitchic module we can expose here - providing instead many seperate SDKs for each service. So we expect the best path is to drop this from the AWS provider in the upcoming major version release. Users who were using it in <6.0.0 versions will need to move to using `@aws-sdk/client-s3` or similar libraries directly.
iwahbe
pushed a commit
that referenced
this pull request
Jul 18, 2023
* Avoid importing full root `@pulumi/aws` module (#2569) Every folder inside the AWS Node.js SDK that has a mixin was importing "../utils", which imports "./awsMixins" which itself was forcing import of "." which then goes back and tries to force import of everything. There is no clear reason why forcing this root module import was ever necesary. Nothing that imports utils.ts depends on the side effects caused by "awsMixins", and the primary effect of adding the `sdk` getter onto the root module is only relevant if the end user loads that root module itself so they can access this property. Also expands some existing test coverage to test this and use of `aws.sdk`, which is the related feature that could in principle be impacted by removing this forced side effect Fixes #772. * Remove aws.sdk property The `aws.sdk` proeprty was introduced several years ago to offer a convenient way for callbacks using function serialization to get ahold of the AWS SDK which is available within the Lambda runtime. However, the AWS SDK v2 that was exposed here is being deprecated, and has been emitted a deprecation warning for some time for anyone using this via Pulumi. The AWS SDK v3 doesn't expose a monolitchic module we can expose here - providing instead many seperate SDKs for each service. So we expect the best path is to drop this from the AWS provider in the upcoming major version release. Users who were using it in <6.0.0 versions will need to move to using `@aws-sdk/client-s3` or similar libraries directly.
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Every folder inside the AWS Node.js SDK that has a mixin was importing "../utils", which imports "./awsMixins" which itself was forcing import of "." which then goes back and tries to force import of everything. There is no clear reason why forcing this root module import was ever necesary. Nothing that imports utils.ts depends on the side effects caused by "awsMixins", and the primary effect of adding the
sdkgetter onto the root module is only relevant if the end user loads that root module itself so they can access this property.Also expands some existing test coverage to test this and use of
aws.sdk, which is the related feature that could in principle be impacted by removing this forced side effectFixes #772.