fix: error in transformers if override has never been setup #1270
Conversation
| const overrideFilePath = path.join(this.overrideConfig!.overrideDir, 'build', 'override.js'); | ||
| if (!_.isEmpty(this.overrideConfig) && this.overrideConfig!.overrideFlag && fs.existsSync(overrideFilePath)) { |
There was a problem hiding this comment.
The fix is in this file. I.e. there's added check if override file exists and condition is pulled to the top of method to spare some useless computing.
| } | ||
| return appsyncResourceObj; | ||
|
|
||
| return {}; |
There was a problem hiding this comment.
if there's no override file or override has not been requested by caller returning {} is consistent with behavior on main branch.
sobolk
requested review from
phani-srikar,
akshbhu,
alharris-at,
awsluja and
jhockett
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## cdkv2 #1270 +/- ##
=======================================
Coverage 63.56% 63.57%
=======================================
Files 302 302
Lines 18993 19005 +12
Branches 4569 4568 -1
=======================================
+ Hits 12073 12082 +9
- Misses 6289 6292 +3
Partials 631 631
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
E2E run with this PR + version bump https://app.circleci.com/pipelines/github/aws-amplify/amplify-category-api/4417/workflows/e2970acc-4cd8-4a9b-ad69-94eaca6ad167 . |
sobolk
changed the title
| stacks.push(node.node.id.split('.')[0]); | ||
| } | ||
| }); | ||
| const overrideFilePath = path.join(this.overrideConfig!.overrideDir, 'build', 'override.js'); |
There was a problem hiding this comment.
we're checking !_.isEmpty(this.overrideConfig) in the below line, but I am afraid this.overrideConfig!.overrideDir will throw if that's the case here.
There was a problem hiding this comment.
@phani-srikar thanks for spotting this. It was working e2e because callers have redundant check.
I refactored those conditionals a bit to simplify the flow. I.e. now non-empty check is bundled with overrideFlag check through ? operator.
I also inverted logic to shortcircuit this method.
No. |
Description of changes
Fixes error that happens on pull.
This was introduced by not swallowing error in previous change. However this scenario has been missed.
Main vs CDKv2 branch:
CDK / CloudFormation Parameters Changed
Issue #, if available
Description of how you validated changes
E2E run with this PR + version bump https://app.circleci.com/pipelines/github/aws-amplify/amplify-category-api/4417/workflows/e2970acc-4cd8-4a9b-ad69-94eaca6ad167 .
The failing
client_e2e_testsalso fail on main due to 3rd party libraries dependency problem.Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.