chore: fix yarn upgrade causing dependency incompatibility#33454
chore: fix yarn upgrade causing dependency incompatibility#33454mergify[bot] merged 2 commits intoaws:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #33454 +/- ##
=======================================
Coverage 81.00% 81.00%
=======================================
Files 238 238
Lines 14271 14271
Branches 2492 2492
=======================================
Hits 11560 11560
Misses 2425 2425
Partials 286 286
Flags with carried forward coverage won't be shown. Click here to find out more.
|
godwingrs22
left a comment
There was a problem hiding this comment.
Great work @samson-keung!. Thanks for the deep dive and for the fix. LGTM
| @@ -93,6 +93,8 @@ | |||
| "nohoist": [ | |||
There was a problem hiding this comment.
Nice. I just came to know about the behaviour of this section. So this makes these listed packages to be present in the individual subpackages (nodemodules within the subpackages) instead of hosting in the root level of the whole projects.
|
Thank you for contributing! Your pull request will be updated from main 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 main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
N/A
The problem was
yarn upgradeno longer worked. You can see the auto upgrade PR - #33299 - is having a failed build.After diving deep into the reason of failure, here are the findings:
I first checked out the branch for #33299, then run the build locally. Here is the error in the build log:
Pay attention to the file paths above.
aws-cdk/node_modules/@types/globis trying to reference a type fromaws-cdk/node_modules/minimatchbecause yarn upgraded to aminimatchversion that natively export minimatch types. But@types/globis not compatible with these newminimatchtypes, causing the error seen above.Ideally,
@types/globshould specify the@types/minimatchversion it works with, but in reality, it has"@types/minimatch": "*", which started pointing to the upgradedaws-cdk/node_modules/minimatchas yarn hoist dependencies into the top levelnode_modules.Some references:
aws-cdk/tools/@aws-cdk/cdk-build-toolsusesmarkdownlint-cli, which depend onglobandminimatchas well.globandminimatchare written in Typescript, which is causing problem when these new version co-exist with the@types/xxxpackages.Description of changes
Use
nohoistfor@types/globand@types/minimatchso that the different places that use these two packages do not conflict with each other at the top levelnode_modules.After doing the above, I noticed
cdk-build-toolswas actually relying on@types/globbut it does not declare the dependency in itspackage.json. It worked because it pulled the@types/globat the top levelnode_modules(which is no longer available withnohoist).Describe any new or updated permissions being added
None
Description of how you validated changes
Locally built and no error.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license