dd-trace-js
dd-trace-js copied to clipboard
Update standard to remove eslint-plugin-node
What does this PR do?
Update to latest version of eslint-config-standard so we can eliminate eslint-plugin-node.
Motivation
eslint-plugin-node triggers vuln scanner warnings so we should get it out of our dependency tree.
Security
Datadog employees:
- [ ] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance. - [x] This PR doesn't touch any of that.
Overall package size
Self size: 6.24 MB Deduped: 61.19 MB No deduping: 61.94 MB
Dependency sizes
| name | version | self size | total size |
|---|---|---|---|
| @datadog/native-iast-taint-tracking | 1.7.0 | 16.71 MB | 16.72 MB |
| @datadog/native-appsec | 7.1.0 | 14.37 MB | 14.38 MB |
| @datadog/pprof | 5.1.0 | 8.83 MB | 9.68 MB |
| protobufjs | 7.2.5 | 2.77 MB | 6.56 MB |
| @datadog/native-iast-rewriter | 2.3.0 | 2.15 MB | 2.24 MB |
| @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB |
| @datadog/native-metrics | 2.0.0 | 898.77 kB | 1.3 MB |
| @opentelemetry/api | 1.4.1 | 780.32 kB | 780.32 kB |
| import-in-the-middle | 1.7.3 | 67.62 kB | 731.01 kB |
| pprof-format | 2.0.7 | 588.12 kB | 588.12 kB |
| msgpack-lite | 0.1.26 | 201.16 kB | 281.59 kB |
| opentracing | 0.14.7 | 194.81 kB | 194.81 kB |
| semver | 7.5.4 | 93.4 kB | 123.8 kB |
| @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB |
| lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB |
| lru-cache | 7.14.0 | 74.95 kB | 74.95 kB |
| ipaddr.js | 2.1.0 | 60.23 kB | 60.23 kB |
| ignore | 5.2.4 | 51.22 kB | 51.22 kB |
| int64-buffer | 0.1.10 | 49.18 kB | 49.18 kB |
| shell-quote | 1.8.1 | 44.96 kB | 44.96 kB |
| istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB |
| tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB |
| limiter | 1.1.5 | 23.17 kB | 23.17 kB |
| dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB |
| retry | 0.13.1 | 18.85 kB | 18.85 kB |
| node-abort-controller | 3.1.1 | 16.89 kB | 16.89 kB |
| jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB |
| crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB |
| path-to-regexp | 0.1.7 | 6.78 kB | 6.78 kB |
| koalas | 1.0.2 | 6.47 kB | 6.47 kB |
| methods | 1.1.2 | 5.29 kB | 5.29 kB |
| module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |
🤖 This report was automatically generated by heaviest-objects-in-the-universe
Codecov Report
Attention: Patch coverage is 50.00000% with 48 lines in your changes are missing coverage. Please review.
Project coverage is 85.06%. Comparing base (
c9e1082) to head (448500a). Report is 1 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #4146 +/- ##
=======================================
Coverage 85.06% 85.06%
=======================================
Files 247 247
Lines 10953 10953
Branches 33 33
=======================================
Hits 9317 9317
Misses 1636 1636
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Benchmarks
Benchmark execution time: 2024-03-20 18:25:39
Comparing candidate commit 448500acb9542e881797069db62878163b5c932e in PR branch update-standard with baseline commit c9e10822c125fb3c42cc0006dd1d8b01fb76fbe4 in branch master.
Found 1 performance improvements and 0 performance regressions! Performance is the same for 258 metrics, 7 unstable metrics.
scenario:plugin-graphql-with-depth-off-18
- 🟩
max_rss_usage[-145.864MB; -82.720MB] or [-15.323%; -8.690%]
Just a thought, but wouldn't it be a much smaller change to adjust the rules to be as they were before instead of changing 306 files? Not against the change per se, but it's very likely to cause conflicts with every open branch, and the goal of the PR is to fix a vulnerability and not to change our rules.
The whole point of using standard is to not debate style issues and just adopt what it says, so if we go and carve out a bunch of exceptions that seems like it's opening up debate about which rules we should or should not follow. Personally I felt it was better to just make the changes to adapt, though I did go for just using a bunch of per-line rule exceptions in a bunch of places just to keep the delta relatively minimal.
If we would prefer to just use standard as a base and build our own ruleset on top of that then we should probably have a discussion here about which of the new rules should be kept and which should be changed. Thoughts?
Apparently I completely missed that #4032 already existed. We could just land that if you prefer how it did things. 🤔
We could just land that if you prefer how it did things.
I don't have a strong opinion. It just felt a bit much to change 300+ files to solve a vulnerability and I would generally prefer this type of change to be as focused as possible. But at the same time, as you said updating might mean additional rules which we would want in the long run anyway. Not blocking either way, it was just an observation.