-
Notifications
You must be signed in to change notification settings - Fork 134
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
BREAKING CHANGE(rum-angular): add support for angular 9 #971
BREAKING CHANGE(rum-angular): add support for angular 9 #971
Conversation
📦 Bundlesize report
|
💔 Build Failed
Expand to view the summary
Build stats
Trends 🧪Steps errorsExpand to view the steps failures
|
1ed2b81
to
293a574
Compare
@chriscarpenter12 If you have some time, can you have a look at the PR. |
Is the plan to get support out for Angular 9 now? Current version is 11 with 12 coming out soon. |
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.
Otherwise looks good. Thanks @vigneshshanmugam !
What will be the process for upgrading apps? Because devs will need to remove the ApmService in their AppModule providers? |
No, I am just doing it for 9 now to keep the changes minimal. We will most likely skip version 9 and go straight to 11 as its the current active version https://angular.io/guide/releases#support-policy-and-schedule. Will do a follow up PR
Still figuring that out, this is not the complete PR though. its just updating the library part. I am going to do follow up PR's on how we can do it for our e2e app and also figure out the update path. |
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.
Thanks @vigneshshanmugam , A few points:
- It seems that this PR breaks the support for version 8, which is fine but we should release this in a breaking change (major) version.
- I think the saucelabs test would give us a better assessment of what might break, so I would prefer if we enable that before merging. We can migrate e2e tests in a separate PR.
|
||
init(config) { | ||
const apmInstance = ApmService.apm.init(config) | ||
const apmInstance = this.ngZone.runOutsideAngular(() => | ||
this.apm.init(config) |
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.
Why does the init has to be out side the zone?
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.
Check the discussion here - #971 (comment)
Yes thats right. We will release a new major for the angular package alone.
We have to change the whole build pipeline for that to even go to that step. We have to convert the |
@jahtalab If there is no major concern on the this PR, I am going to go ahead and merge and address some of the concerns on the next E2E test PR which would make things easier to understand and answer some of your questions as well. Makes my iteration faster. |
@vigneshshanmugam , I would like to run this locally but without the tests it's a bit hard to verify the code actually does what it's supposed to do. Would it help you if we make an intermediary branch to merge this into? This has the added benefit of being able to test the whole feature locally without breaking the main branch. |
I've created |
Had a offline discussion, we will merge all Angular related changes in to the |
* feat(rum-angular): add support for angular 9 * chore: set up the ng build toolchain * feat: rewrite the apm module under src/* * chore: set up the testing with ng * chore: make karma:dev work * chore: make karma:coverage work * chore: address review and disable saucelabs * chore: fix local link and coverage test
* feat(rum-angular): add support for angular 9 * chore: set up the ng build toolchain * feat: rewrite the apm module under src/* * chore: set up the testing with ng * chore: make karma:dev work * chore: make karma:coverage work * chore: address review and disable saucelabs * chore: fix local link and coverage test
* feat(rum-angular): add support for angular 9 * chore: set up the ng build toolchain * feat: rewrite the apm module under src/* * chore: set up the testing with ng * chore: make karma:dev work * chore: make karma:coverage work * chore: address review and disable saucelabs * chore: fix local link and coverage test
* feat(rum-angular): add support for angular 9 * chore: set up the ng build toolchain * feat: rewrite the apm module under src/* * chore: set up the testing with ng * chore: make karma:dev work * chore: make karma:coverage work * chore: address review and disable saucelabs * chore: fix local link and coverage test
* feat(rum-angular): add support for angular 9 * chore: set up the ng build toolchain * feat: rewrite the apm module under src/* * chore: set up the testing with ng * chore: make karma:dev work * chore: make karma:coverage work * chore: address review and disable saucelabs * chore: fix local link and coverage test
* feat(rum-angular): add support for angular 9 * chore: set up the ng build toolchain * chore: rewrite the apm module under src/* * chore: set up the testing with ng * chore: make karma:dev work * chore: make karma:coverage work * chore: address review and disable saucelabs * chore: fix local link and coverage test
* feat(rum-angular): add support for angular 9 * chore: set up the ng build toolchain * chore: rewrite the apm module under src/* * chore: set up the testing with ng * chore: make karma:dev work * chore: make karma:coverage work * chore: address review and disable saucelabs * chore: fix local link and coverage test
* feat(rum-angular)!: angular 9-11 support (#971) * feat(rum-angular): add support for angular 9 * chore: set up the ng build toolchain * chore: rewrite the apm module under src/* * chore: set up the testing with ng * chore: make karma:dev work * chore: make karma:coverage work * chore: address review and disable saucelabs * chore: fix local link and coverage test * chore: migrate angular e2e app to ng cli (#974) * chore: migrate angular e2e app to ng cli * chore: make build system work for e2e * chore: make saucelabs work * chore: enable saucelabs test for angular * chore: fix test scripts * chore: target es5 and add browserlist * chore: only run saucelabs test in jenkins * chore: update prod apmserver url * chore: transpile apmservermock correctly * chore: revert jenkinsfile * chore: fix warnings in build * chore(rum-angular): fix build warnings in prod * chore(rum-angular): add prepublish command * chore(release): publish angular - @elastic/apm-rum-angular@2.0.0-alpha.0 * chore(release): publish angular - @elastic/apm-rum-angular@2.0.0-alpha.1 * docs: update angular integration for >9 versions (#1000) * chore: update angular to latest 11.2.5 (#996) * chore(release): publish angular - @elastic/apm-rum-angular@2.0.0-alpha.2 * chore: fix license lint issue
* feat(rum-angular)!: angular 9-11 support (elastic#971) * feat(rum-angular): add support for angular 9 * chore: set up the ng build toolchain * chore: rewrite the apm module under src/* * chore: set up the testing with ng * chore: make karma:dev work * chore: make karma:coverage work * chore: address review and disable saucelabs * chore: fix local link and coverage test * chore: migrate angular e2e app to ng cli (elastic#974) * chore: migrate angular e2e app to ng cli * chore: make build system work for e2e * chore: make saucelabs work * chore: enable saucelabs test for angular * chore: fix test scripts * chore: target es5 and add browserlist * chore: only run saucelabs test in jenkins * chore: update prod apmserver url * chore: transpile apmservermock correctly * chore: revert jenkinsfile * chore: fix warnings in build * chore(rum-angular): fix build warnings in prod * chore(rum-angular): add prepublish command * chore(release): publish angular - @elastic/apm-rum-angular@2.0.0-alpha.0 * chore(release): publish angular - @elastic/apm-rum-angular@2.0.0-alpha.1 * docs: update angular integration for >9 versions (elastic#1000) * chore: update angular to latest 11.2.5 (elastic#996) * chore(release): publish angular - @elastic/apm-rum-angular@2.0.0-alpha.2 * chore: fix license lint issue
metadata.json
as we get those for free using the ng toolschain.@elastic/apm-rum-angular
package.ApmModule
ApmService
ApmErrorHandler
ng
ecosystem which is the recommended way to build, test and publish the angular libraries and application.ng-packgr
is used for building the library - checkng-package.json
angular.json
which has specifictsconfig
respectively.Note to Reviewer: Review commit by commit for better understanding
Tasks to be done in separate PR
These tasks are already added to the main issue #962 to keep changes minimal and moving faster with iteration.