fix(aws-custom-resource): module fails loading when bundled with parcel #11487
Conversation
|
|
| @@ -435,6 +429,7 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { | |||
| */ | |||
| function awsSdkToIamAction(service: string, action: string): string { | |||
| const srv = service.toLowerCase(); | |||
| const awsSdkMetadata = getAwsSdkMetadata(); | |||
There was a problem hiding this comment.
We should cache this call, otherwise we are going to read and parse for every action in every AwsCustomResource
There was a problem hiding this comment.
Updated in revision.
| @@ -435,6 +430,7 @@ export class AwsCustomResource extends cdk.Construct implements iam.IGrantable { | |||
| */ | |||
| function awsSdkToIamAction(service: string, action: string): string { | |||
| const srv = service.toLowerCase(); | |||
| awsSdkMetadata = getAwsSdkMetadata(); | |||
There was a problem hiding this comment.
I do not like that the cache variable is written in one function and read in another. Responsibilities should be more clearly divided than that.
Please put the awsSdkMetadata global variable close to the getAwsSdkMetadata() function in the source, and make sure it's only referenced from that function. If you want to get fancy you can make it a closure, but just co-locating in the source, and maybe renaming to _awsSdkMetadataCache or something should inform people no other part of the code should be touching it.
After that, in places where it needs to be consumed (only here?), assing the result to a local variable and use that.
rix0rrr
changed the title
This change makes getting the metadata a function instead of a const, fixes aws#7284. Since this change only impacted the way in which awsSdkMetadata was provided to awsSdkToIamAction no new tests were warranted, but I reran yarn build and all tests stull passed.
… called for every AwsCustomResource
TheRealAmazonKendra
force-pushed
the
master
branch
from
d33d201
to
9e7f7e2
Compare
mergify
bot
dismissed
rix0rrr’s stale review
Pull request has been modified.
rix0rrr
added
the
pr-linter/exempt-test
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
After this change,
aws-custom-resourcewill still not work properly when bundled using parcel, but at least the module won't throw an error anymore when it's being loaded.Fixes #7284.
Since this change only impacted the way in which awsSdkMetadata was provided to awsSdkToIamAction no new tests were warranted, but I re-ran yarn build and all tests still passed.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license