-
Notifications
You must be signed in to change notification settings - Fork 306
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
vendor jsonpath-plus #4785
vendor jsonpath-plus #4785
Conversation
Overall package sizeSelf size: 7.54 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/native-appsec | 8.1.1 | 18.67 MB | 18.68 MB | | @datadog/native-iast-taint-tracking | 3.1.0 | 12.27 MB | 12.28 MB | | @datadog/pprof | 5.3.0 | 9.85 MB | 10.22 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.5.0 | 2.51 MB | 2.59 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.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 826.22 kB | | msgpack-lite | 0.1.26 | 201.16 kB | 281.59 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | semver | 7.6.3 | 95.82 kB | 95.82 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | lru-cache | 7.14.0 | 74.95 kB | 74.95 kB | | ignore | 5.3.1 | 51.46 kB | 51.46 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 | | rfdc | 1.3.1 | 25.21 kB | 25.21 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 | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | path-to-regexp | 0.1.10 | 6.38 kB | 6.38 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 |
BenchmarksBenchmark execution time: 2024-10-16 03:43:12 Comparing candidate commit 1109522 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 260 metrics, 6 unstable metrics. |
We need the latest version (10.0.0) so that it doesn't have vulnerabilities, but we need it to be compatible with Node.js 16.0.0, so we needed to vendor it and make slight adjustments.
96ad527
to
594b9cf
Compare
// NOTE(bengl): This file is taken directly from jsonpath-plus@10.0.0 | ||
// | ||
// https://github.com/JSONPath-Plus/JSONPath/blob/a04dcbac760fed48760b09f387874a36f289c3f3/dist/index-node-cjs.index-node-cjs | ||
// | ||
// The only changes are: | ||
// - Replace Object.hasOwn with polyfill |
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 would either add a short description of the reason and link to this PR or mention that we can revert this inlining once we drop support for Node.js 16
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 also realized that vendoring is distribution, and distribution requires the inclusion of the original license (as the license dictates), so I've included that, and a bunch more detail, in the latest rev.
* vendor jsonpath-plus We need the latest version (10.0.0) so that it doesn't have vulnerabilities, but we need it to be compatible with Node.js 16.0.0, so we needed to vendor it and make slight adjustments. * more clarity in comment, and add the license
* vendor jsonpath-plus We need the latest version (10.0.0) so that it doesn't have vulnerabilities, but we need it to be compatible with Node.js 16.0.0, so we needed to vendor it and make slight adjustments. * more clarity in comment, and add the license
Is the whole functionality of every possible syntax even needed or is our use case actually smaller and could be achieved more simply? |
We need the latest version (10.0.0) so that it doesn't have vulnerabilities, but we need it to be compatible with Node.js 16.0.0, so we needed to vendor it and make slight adjustments.