-
Notifications
You must be signed in to change notification settings - Fork 2
chore: upgrade ng from 11 to 12 #302
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
Conversation
This comment has been minimized.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## main #302 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 49 49
Lines 655 655
Branches 62 62
=========================================
Hits 655 655 Continue to review full report at Codecov.
|
"@angular/forms": "^12.2.1", | ||
"@angular/platform-browser": "^12.2.1", | ||
"@angular/platform-browser-dynamic": "^12.2.1", | ||
"@angular/router": "^12.2.1", |
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.
what about zone.js? does it need to be upgraded?
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.
No, I think we're already on at latest one https://www.npmjs.com/package/zone.js?activeTab=versions
@@ -14,4 +14,4 @@ export const environment = { | |||
* This import should be commented out in production mode because it will have a negative impact | |||
* on performance if an error is thrown. | |||
*/ | |||
// import 'zone.js/dist/zone-error'; // Included with Angular CLI. | |||
// import 'zone.js/plugins/zone-error'; // Included with Angular CLI. |
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 see this change here but no change in zone.js dependency. Is this right?
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.
Yeah, answered above.
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.
above you answered that zone.js doesn't need to be upgraded - but I think anand is asking: then why would this change? was it previously wrong?
It looks like the zone docs agree that it was:
https://github.com/angular/angular/tree/master/packages/zone.js#bundles
@itssharmasandeep quick question: What is peer dependency in npm? |
@@ -14,4 +14,4 @@ export const environment = { | |||
* This import should be commented out in production mode because it will have a negative impact | |||
* on performance if an error is thrown. | |||
*/ | |||
// import 'zone.js/dist/zone-error'; // Included with Angular CLI. | |||
// import 'zone.js/plugins/zone-error'; // Included with Angular CLI. |
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.
above you answered that zone.js doesn't need to be upgraded - but I think anand is asking: then why would this change? was it previously wrong?
It looks like the zone docs agree that it was:
https://github.com/angular/angular/tree/master/packages/zone.js#bundles
We need to upgrade peer dependency before merging this pr. i have shared reading material with sandeep on slack |
Co-authored-by: Aaron Steinfeld <[email protected]>
This comment has been minimized.
This comment has been minimized.
right - forgot this repo even had the separate project |
taking |
Unit Test Results 2 files 15 suites 24s ⏱️ Results for commit 84b0544. |
why? without the peer dependencies this isn't usable (so shouldn't have been merged). what am I missing? |
@aaron-steinfeld I want sandeep to try out and learn the entire process. Explained him few things, but a good hands on would help him learn the entire flow. So, I asked him to merge this PR and see what is breaking in ht-ui when he tries to add the new bundle. |
🎉 This PR is included in version 2.6.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Due to some problem with #297
This is the new one with addressed comments