Skip to content

Commit 1a1863d

Browse files
committed
esbuild: graceful continue when bundling dead code (#3215)
- previously, when encountering a dead code path that requires a not-installed instrumented package, build would fail - this would happen when, say, `knex` requires the `tedious` library for an app that is only making use of `pg` - without `dd-trace/esbuild`, a user simply adds `tedious` to their `external` list and goes on with their day - or in other words, vanilla esbuild doesn't really care when it encounters these missing modules - with `dd-trace/esbuild`, we would throw an error and the build fails - one solution would be to not instrument external packages but many users expect this behavior to work - in fact, we've been telling users to do just this before we supported a plugin - now, with this change, the `require('unused-module')` call remains in the output code - print a warning when this happens (at build time), regardless of debug level, since it might not be intentional
1 parent 1148862 commit 1a1863d

File tree

6 files changed

+30
-3
lines changed

6 files changed

+30
-3
lines changed

LICENSE-3rdparty.csv

+1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ dev,glob,ISC,Copyright Isaac Z. Schlueter and Contributors
5151
dev,graphql,MIT,Copyright 2015 Facebook Inc.
5252
dev,int64-buffer,MIT,Copyright 2015-2016 Yusuke Kawasaki
5353
dev,jszip,MIT,Copyright 2015-2016 Stuart Knightley and contributors
54+
dev,knex,MIT,Copyright (c) 2013-present Tim Griesser
5455
dev,mkdirp,MIT,Copyright 2010 James Halliday
5556
dev,mocha,MIT,Copyright 2011-2018 JS Foundation and contributors https://js.foundation
5657
dev,multer,MIT,Copyright 2014 Hage Yaapa

integration-tests/esbuild/basic-test.js

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const tracer = require('../../').init() // dd-trace
1111
const assert = require('assert')
1212
const express = require('express')
1313
const http = require('http')
14+
require('knex') // has dead code paths for multiple instrumented packages
1415

1516
const app = express()
1617
const PORT = 31415

integration-tests/esbuild/build.js

+12-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,18 @@ esbuild.build({
99
outfile: 'out.js',
1010
plugins: [ddPlugin],
1111
platform: 'node',
12-
target: ['node16']
12+
target: ['node16'],
13+
external: [
14+
// dead code paths introduced by knex
15+
'pg',
16+
'mysql2',
17+
'better-sqlite3',
18+
'sqlite3',
19+
'mysql',
20+
'oracledb',
21+
'pg-query-stream',
22+
'tedious'
23+
]
1324
}).catch((err) => {
1425
console.error(err) // eslint-disable-line no-console
1526
process.exit(1)

integration-tests/esbuild/package.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"license": "ISC",
2020
"dependencies": {
2121
"esbuild": "0.16.12",
22-
"express": "^4.16.2"
22+
"express": "^4.16.2",
23+
"knex": "^2.4.2"
2324
}
2425
}

package.json

+1
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@
120120
"graphql": "0.13.2",
121121
"int64-buffer": "^0.1.9",
122122
"jszip": "^3.5.0",
123+
"knex": "^2.4.2",
123124
"mkdirp": "^0.5.1",
124125
"mocha": "8",
125126
"msgpack-lite": "^0.1.26",

packages/datadog-esbuild/index.js

+13-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,19 @@ module.exports.setup = function (build) {
3939

4040
if (args.namespace === 'file' && packagesOfInterest.has(packageName)) {
4141
// The file namespace is used when requiring files from disk in userland
42-
const pathToPackageJson = require.resolve(`${packageName}/package.json`, { paths: [ args.resolveDir ] })
42+
43+
let pathToPackageJson
44+
try {
45+
pathToPackageJson = require.resolve(`${packageName}/package.json`, { paths: [ args.resolveDir ] })
46+
} catch (err) {
47+
if (err.code === 'MODULE_NOT_FOUND') {
48+
console.warn(`Unable to open "${packageName}/package.json". Is the "${packageName}" package dead code?`)
49+
return
50+
} else {
51+
throw err
52+
}
53+
}
54+
4355
const pkg = require(pathToPackageJson)
4456

4557
if (DEBUG) {

0 commit comments

Comments
 (0)