-
Notifications
You must be signed in to change notification settings - Fork 825
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
feat(minification): Add noEmitHelpers, importHelpers and tslib as a dependency #3914
Conversation
experimental/packages/opentelemetry-instrumentation/src/platform/browser/index.ts
Show resolved
Hide resolved
experimental/packages/opentelemetry-instrumentation/test/node/EsmInstrumentation.test.mjs
Show resolved
Hide resolved
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3914 +/- ##
=======================================
Coverage 92.89% 92.89%
=======================================
Files 297 297
Lines 8838 8838
Branches 1815 1815
=======================================
Hits 8210 8210
Misses 628 628 |
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.
Looks good to me 🙂
Just wondering about the change in the esm instrumentation test. Maybe @JamieDanielson can shed some light on why this might be needed 🤔
experimental/packages/opentelemetry-instrumentation/test/node/EsmInstrumentation.test.mjs
Show resolved
Hide resolved
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 like this change! Just a minor nit about the change. |
8cefd1c
to
1e0c4a0
Compare
…ependency (open-telemetry#3914) * feat(minification): Add noEmitHelpers, importHelpers and tslib as a dependency * fix: Lint fixes * Remove noEmitHelpers as not needed --------- Co-authored-by: Chengzhong Wu <[email protected]>
…b as a dependency (open-telemetry#3914)" This reverts commit 74393ac.
Which problem is this PR solving?
To better support minification of the generated code we need to change the default settings by adding the following to the tsconfg.base.json.
Fixes #3913
Short description of the changes
Adds
importHelpers
andnoEmitHelpers
totsconfig.base.json
Updates all
package.json
to includetslib
as a dependency with "^2.3.1" as this version is the first version that supported TypeScript 4.4, this should allow all v2.x versions oftslib
to be used.Updates instrumentation test
EsmInstrumentation.test.mjs
due to failure to importInstrumentationBase
Type of change
Please delete options that are not relevant.
This is not expected to be a code breaking change
How Has This Been Tested?
Compiled and built locally using
npm run compile
andnpm test
Checklist: