-
Notifications
You must be signed in to change notification settings - Fork 805
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
chore(opentelemetry-context-zone-peer-dep): support zone.js ^v0.13.0 #4320
chore(opentelemetry-context-zone-peer-dep): support zone.js ^v0.13.0 #4320
Conversation
update deps to support angular 16 closes issue open-telemetry#4245 Signed-off-by: rbrennan <4884521+rodgerbrennan@users.noreply.github.com>
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.
Hi there, thank you for your contribution!
Looks like a few unintentional changes made it in here, I pointed them out in the comments. 🙂
This reverts commit 624df4e.
464a72c
to
ea1e5c8
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4320 +/- ##
=======================================
Coverage 92.20% 92.20%
=======================================
Files 336 336
Lines 9512 9512
Branches 2016 2016
=======================================
Hits 8771 8771
Misses 741 741 |
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.
FYI @rodgerbrennan I'm not sure that you're aware, but the browser tests for this PR seem to be failing - you can invoke them locally by running npm run compile
folllowed by npm run test:browser
.
We'll need the tests need to pass before we can merge this PR to ensure we don't introduce a regression for our users.
@pichlermarc I believe my latest commit fixes the browser tests |
@@ -14,7 +14,7 @@ | |||
* limitations under the License. | |||
*/ | |||
|
|||
import 'zone.js'; | |||
import 'zone.js/dist/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.
I believe we should enable transpiling the zone.js with webpack if the default entry of zone.js contains advanced syntax.
This can be done in a follow-up PR.
import 'zone.js/dist/zone'; | |
// Default 'zone.js' contains advanced syntax that can not be consumed by the test webpack. | |
// Load the es5 target bundle instead. | |
import 'zone.js/dist/zone'; |
@rodgerbrennan Still requires an entry in |
I added a note in the changelog under unreleased, house(internal) |
…pen-telemetry#4320) * chore(opentelemetry-context-zone-peer-dep): update deps to support angular 16 closes issue open-telemetry#4245 Signed-off-by: rbrennan <4884521+rodgerbrennan@users.noreply.github.com> * update packages * Revert "update packages" This reverts commit 624df4e. * chore: sync package-lock.json * chore: sync package-lock.json * chore: sync package-lock.json * Update ZoneContextManager.test to use ES5 UMD bundle * add changelog * remove trailing space from changelog --------- Signed-off-by: rbrennan <4884521+rodgerbrennan@users.noreply.github.com> Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>
update deps to support angular 16
Fixes #4245