-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(ng-update): material update fails due to circular dependency #16538
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(ng-update): material update fails due to circular dependency #16538
Conversation
Currently running `ng update` for both Material and CDK fails because there is a circular dependency in the CDK schematics that cause the update-tool exports to be not available to the Material ng-update schematic. Without putting to much effort into ensuring no circular dependencies, this commit also sets up Madge to avoid such issues in the future.
@@ -312,6 +312,10 @@ jobs: | |||
- run: yarn gulp ci:build-release-packages | |||
- run: yarn check-release-output | |||
|
|||
# TODO(devversion): replace this with bazel tests that run Madge. This is | |||
# cumbersome and doesn't guarantee no circular deps for other entry-points. | |||
- run: yarn madge --circular dist/releases/cdk/schematics/index.js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to just add this one-liner as that command helped me finding the circular dependency that broke the Material update. Long-term solution should definitely be different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
) Currently running `ng update` for both Material and CDK fails because there is a circular dependency in the CDK schematics that cause the update-tool exports to be not available to the Material ng-update schematic. Without putting to much effort into ensuring no circular dependencies, this commit also sets up Madge to avoid such issues in the future.
To clarify: I did various manual testings after the ng-update rework but such issues are not easy to test with packages which are not published on NPM as Ideally we'll have some real integration tests for this. And next time I'll make sure to push the schematics to a NPM package for manual testing (even though it's way more effort..) |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Currently running
ng update
for both Material and CDK fails because thereis a circular dependency in the CDK schematics that cause the update-tool
exports to be not available to the Material ng-update schematic.
Without putting to much effort into ensuring no circular dependencies, this
commit also sets up Madge to avoid such issues in the future.