dd-trace-js
dd-trace-js copied to clipboard
Use files property in package.json instead of .npmignore
What does this PR do?
Delete .npmignore and use the files property in package.json to select the files to include in the published npm package.
Motivation
The n/no-unpublished-require ESLint rule incorrectly flagged some require calls from the root of our project into the packages directory as referencing files not published to npm. This was a bug in that rule.
After researching how we select files to be published or not, I found that we use .npmignore as an allowlist instead of a denylist - which it is normally meant to be used as. Thinking this might somehow trigger the bug, I deleted the .npmignore file and moved the allowlist to the files property in package.json and that fixed the problem.
Plugin Checklist
- [ ] Unit tests.
- [ ] TypeScript definitions.
- [ ] TypeScript tests.
- [ ] API documentation.
- [ ] CircleCI jobs/workflows.
- [ ] Plugin is exported.
Additional Notes
I've verified the package produced by npm pack between this branch and master and the the only difference, is that the package produced on this branch doesn't include packages/memwatch/package.json. However, I think that was a mistake to include in the first place, so I'd say this change is actually an improvement.
- #5320
👈 (View in Graphite) - #5216
: 4 other dependent PRs (#5218
, #5274
, #5334
and 1 other) - #6027

- #6026

master
This stack of pull requests is managed by Graphite. Learn more about stacking.
Overall package size
Self size: 9.61 MB Deduped: 108.63 MB No deduping: 109.01 MB
Dependency sizes
| name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.7.0 | 35.02 MB | 35.02 MB | | @datadog/native-appsec | 9.0.0 | 19.6 MB | 19.61 MB | | @datadog/native-iast-taint-tracking | 4.0.0 | 11.72 MB | 11.73 MB | | @datadog/pprof | 5.9.0 | 9.77 MB | 10.14 MB | | @opentelemetry/core | 1.30.1 | 908.66 kB | 7.16 MB | | protobufjs | 7.5.3 | 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.8.0 | 1.21 MB | 1.21 MB | | jsonpath-plus | 10.3.0 | 617.18 kB | 1.08 MB | | import-in-the-middle | 1.14.2 | 122.36 kB | 850.93 kB | | lru-cache | 10.4.3 | 804.3 kB | 804.3 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | limiter | 3.0.0 | 157.92 kB | 157.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 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 | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | dc-polyfill | 0.1.9 | 25.11 kB | 25.11 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | shell-quote | 1.8.3 | 23.74 kB | 23.74 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 | | koalas | 1.0.2 | 6.47 kB | 6.47 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 :white_check_mark:
Project coverage is 81.85%. Comparing base (
d4c55ba) to head (3ed3382). Report is 2 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #5320 +/- ##
==========================================
+ Coverage 81.78% 81.85% +0.07%
==========================================
Files 471 472 +1
Lines 19310 19431 +121
==========================================
+ Hits 15792 15906 +114
- Misses 3518 3525 +7
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Datadog Report
Branch report: watson/npmignore2
Commit report: d9bed6f
Test service: dd-trace-js-integration-tests
:x: 138 Failed (138 Known Flaky), 1015 Passed, 0 Skipped, 20m 49.45s Total Time
:x: Failed Tests (138)
This report shows up to 5 failed tests.
-
jest CommonJS can report git metadata-integration-tests/jest/jest.spec.js- :snowflake: Known flaky - DetailsExpand for error
imeout -
jest CommonJS can report git metadata-integration-tests/jest/jest.spec.js- :snowflake: Known flaky - DetailsExpand for error
imeout -
jest CommonJS can run and report tests with agentless-integration-tests/jest/jest.spec.js- :snowflake: Known flaky - DetailsExpand for error
imeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/runner/work/dd-trace-js/dd-trace-js/integration-tests/jest/jest.spec.js) -
jest CommonJS can run and report tests with agentless-integration-tests/jest/jest.spec.js- :snowflake: Known flaky - DetailsExpand for error
imeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/runner/work/dd-trace-js/dd-trace-js/integration-tests/jest/jest.spec.js) -
jest CommonJS can run and report tests with evp proxy-integration-tests/jest/jest.spec.js- :snowflake: Known flaky - DetailsExpand for error
imeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/runner/work/dd-trace-js/dd-trace-js/integration-tests/jest/jest.spec.js)
Benchmarks
Benchmark execution time: 2025-07-09 10:27:04
Comparing candidate commit 3ed3382023a54a122e260d20926ab71754abaa4e in PR branch watson/npmignore2 with baseline commit d4c55ba45235ccfe9e6735d5deb8b8025efbb813 in branch master.
Found 0 performance improvements and 0 performance regressions! Performance is the same for 1165 metrics, 48 unstable metrics.
Looks pretty good, always better to have whitelist than blacklist. Are we sure the memwatch thingy should be excluded ? Another thing i'm thinking is, if we forget to include files in the package, we might not realize before it's already released, since I think our CI passes tests on the repo code, not the package code right ?
@simon-id Please read the PR description ;) All your questions are already addressed there, but the TLDR is:
Are we sure the memwatch thingy should be excluded ?
Pretty sure it was a bug to include it before
Another thing i'm thinking is, if we forget to include files in the package, we might not realize before it's already released, since I think our CI passes tests on the repo code, not the package code right ?
You're right, but this PR doesn't change that issue, since we always has used the .npmignore file as if it was a files array in pacakge.json.
You can argue that we should make a CI check for this, but that's unrelated to this PR.
Yeah I read the description already, but how can we be sure it was a bug in the first place you know ? i don't even know what that memwatch thing does :shrug: And as for the second point you're right, i got confused by the double negation, npmignore+negative asserts == same thing as files array yes :+1:
I added a commit which includes all files automatically added by npm, which technically doesn't have to be included in the files list. But based on review feedback it has been decided to be explicit as to not rely on knowledge of what npm includes to know what will be included.