-
Notifications
You must be signed in to change notification settings - Fork 350
add support for test version dependency resolutions #6561
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
Conversation
Overall package sizeSelf size: 12.48 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.7.0 | 35.02 MB | 35.02 MB | | @datadog/native-appsec | 10.2.1 | 20.64 MB | 20.65 MB | | @datadog/native-iast-taint-tracking | 4.0.0 | 11.72 MB | 11.73 MB | | @datadog/pprof | 5.10.0 | 9.91 MB | 10.3 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.5.4 | 2.95 MB | 5.6 MB | | @datadog/wasm-js-rewriter | 4.0.1 | 2.85 MB | 3.58 MB | | @datadog/native-metrics | 3.1.1 | 1.02 MB | 1.43 MB | | @opentelemetry/api | 1.9.0 | 1.22 MB | 1.22 MB | | jsonpath-plus | 10.3.0 | 617.18 kB | 1.08 MB | | import-in-the-middle | 1.14.4 | 123.18 kB | 851.76 kB | | lru-cache | 10.4.3 | 804.3 kB | 804.3 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | source-map | 0.7.6 | 185.63 kB | 185.63 kB | | pprof-format | 2.2.1 | 163.06 kB | 163.06 kB | | @datadog/sketches-js | 2.1.1 | 109.9 kB | 109.9 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 7.0.5 | 63.38 kB | 63.38 kB | | istanbul-lib-coverage | 3.2.2 | 34.37 kB | 34.37 kB | | rfdc | 1.4.1 | 27.15 kB | 27.15 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | shell-quote | 1.8.3 | 23.74 kB | 23.74 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | mutexify | 1.4.0 | 5.71 kB | 8.74 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | module-details-from-path | 1.0.4 | 3.96 kB | 3.96 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6561 +/- ##
=======================================
Coverage 83.90% 83.90%
=======================================
Files 485 485
Lines 20337 20337
=======================================
Hits 17063 17063
Misses 3274 3274 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
BenchmarksBenchmark execution time: 2025-09-30 23:39:28 Comparing candidate commit d8fe7d0 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1607 metrics, 63 unstable metrics. |
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 reached out to Jordan and I guess he has an interest to unbreak koa. Let's wait and see until tomorrow
917342b to
2b15471
Compare
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.
Things are working again. I don't think we should pin it therefore.
2b15471 to
d8fe7d0
Compare
Agreed, but I think it makes sense to have support for resolutions if we need them for another similar issue. |
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 am not really convinced about adding this while we do not use it. If this comes up again, I think we should look at it in a similar way as we did anyway and adding functionality that is not used is not good for me.
I agree in principle, but I'd rather have the code already there and ready whenever we need it than not, or having to research this again by someone who doesn't know about this PR. Let's not forget we often get weird situations that can be surprising and sometimes difficult to address, so any amount of readiness already built-in is a good thing, especially when we know it happened in the past. |
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 do not think anyone would find this with the current suggestion. If you or me would face a similar situation, we will probably remember either way. Another person won't know about it and won't find it and has to invest the same time due to that.
With that in mind, I do not think we should land this.
|
Wouldn't adding it to the |
Unfortunately resolutions don't work at the workspace level. At the end of the day, the right solution is to lock all transitive dependencies, but that's more work that I don't have time to do right now, but I'll try to take time in the following weeks. |
|
Let's revisit this later as a long-term solution would be best. |
What does this PR do?
Add support for test version dependency resolutions.
Motivation
A new version of
is-generator-function(1.1.1) was released which depends ongenerator-function(2.0.0) which breaks Koa 2.x completely, so we need a way to pin the underlying dependency.Additional Notes
Ideally we'd be able to pin all transitive dependencies always through something like a lockfile per test version, but that would require a larger change, so for now this workaround will have to do.