fix: enable downlevelIteration for es5 targets #2823
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2823 +/- ##
=======================================
Coverage 93.52% 93.52%
=======================================
Files 163 163
Lines 5572 5572
Branches 1180 1180
=======================================
Hits 5211 5211
Misses 361 361 |
legendecas
force-pushed
the
metrics-ff/ci
branch
2 times, most recently
from
0e747b9
to
f439147
Compare
| @@ -1,6 +1,7 @@ | |||
| { | |||
| "extends": "./tsconfig.base.json", | |||
| "compilerOptions": { | |||
| "target": "es5" | |||
| "target": "es5", | |||
| "downlevelIteration": true | |||
There was a problem hiding this comment.
Though, Map/Set is available since ES2015. TypeScript doesn't introduce any polyfills for Map/Set. I think the es5-esm products we are publishing right now do not actually work for es5 environments without specific setups.
The good news is that the es5-esm products do not contain any fashion syntaxes (other than the ESM part) so that end-users do not need to transpile the otel-packages. They just need to include the polyfill packages. I believe this doesn't break the assumptions #2472 (comment).
legendecas
force-pushed
the
metrics-ff/ci
branch
from
f439147
to
dae0e56
Compare
legendecas
changed the title
legendecas
changed the title
There was a problem hiding this comment.
I believe all the duplicate compiles will drastically slow down our CI. Packages like core which are dependency of many packages may be compiled 10s of times if not over 100 times. Most of these compiles will be no-op because of incremental builds, but it is still quite some overhead.
| @@ -76,6 +76,8 @@ jobs: | |||
| - name: Build 🔧 | |||
| run: | | |||
| npm run compile | |||
| # run additional compilation variants | |||
There was a problem hiding this comment.
This is going to duplicate compile many many times because each package you run compile in also compiles its dependencies. This is why the compile is done from npm run compile to ensure everything is only compiled a single time.
There was a problem hiding this comment.
The time spent is about 6minutes compared to about 2minutes before. And the full-compilation only happens in browser-tests -- Node.js tests only run the root tsconfig.json build.
Since these products are going to be released, we need to make sure the releasing variant is working so I think this is acceptable, WDYT?
There was a problem hiding this comment.
ah sorry I thought I already responded here. I think it's fine.
Which problem is this PR solving?
The root tsconfig.json only compiles the basic, Node.js target product with the fix of #2765. Other compilation variants should also be tested in GitHub Actions.
Fixes #2821 (comment)
Type of change
Checklist:
Unit tests have been added