-
Notifications
You must be signed in to change notification settings - Fork 532
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: update lerna to v7 and migrate to npm workspace #1561
chore: update lerna to v7 and migrate to npm workspace #1561
Conversation
the default node-14 env is bundle with npm@6's which seem not support npm workspace. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1561 +/- ##
==========================================
- Coverage 96.06% 91.77% -4.29%
==========================================
Files 14 137 +123
Lines 914 7064 +6150
Branches 199 1424 +1225
==========================================
+ Hits 878 6483 +5605
- Misses 36 581 +545
|
…trib into feat-learna-bootstrap-deprecate
This reverts commit 0caf8a9.
nx@16.5.0 are using nodejs's global performance object, which introduced in node@16. this cause nx fail on node@14 PS: this is fixed with nx@16.5.1 |
Most of the issue is fixed, the issue with browser test will be fix by #1565 |
I will need some more time to test for tav. |
…1/opentelemetry-js-contrib into feat-learna-bootstrap-deprecate
@chigia001 why did this get closed and not merged in? |
@DanielLoyAugmedix I have a new pr for this but still WIP |
Which problem is this PR solving?
lerna bootstrap
,lerna link
, andlerna add
, we must depend on the package's manager native solutionFixes Local build differs from CI build #904
Fixes Update dependency lerna to
v7
#1542Short description of the changes
gts
as it does not use anymore; I just happened to see this because its part of thelerna bootstrap
arguments@types/sinon<10.0.14
depend on@sinonjs/fake-timers
which install a very old version of sinon => this cause some compile issuezone.js
=> instrumentation-user-interaction depends on zone.js's type, but the way the type is defined in zone.js is generated, it can't be used with the import statement. We must use/// <reference
syntax so that we don't need to fix the path tozone.js/dist/zone.js.d.ts
yarn bootstrap
anymore, the previous best practice for node lerna cache not apply for this caseThere might be some hoist issues in the future, and the current npmcli script doesn’t provide enough support for us for more complex hoisting like what
lerna
used to provide.Next Step
As lerna@7 is just provide some additional feature on top of nx, ee might consider to include nx.json to increase caching between task and apply for nx-cloud. Nx-cloud should be free for OSS project like opentelemetry-js-contrib. Those change might also fix issue with #1473