Skip to content

Commit

Permalink
esbuild: graceful continue when bundling dead code (#3215)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
tlhunter authored Jun 5, 2023
1 parent 0144fb3 commit 5082455
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 3 deletions.
1 change: 1 addition & 0 deletions LICENSE-3rdparty.csv
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ dev,glob,ISC,Copyright Isaac Z. Schlueter and Contributors
dev,graphql,MIT,Copyright 2015 Facebook Inc.
dev,int64-buffer,MIT,Copyright 2015-2016 Yusuke Kawasaki
dev,jszip,MIT,Copyright 2015-2016 Stuart Knightley and contributors
dev,knex,MIT,Copyright (c) 2013-present Tim Griesser
dev,mkdirp,MIT,Copyright 2010 James Halliday
dev,mocha,MIT,Copyright 2011-2018 JS Foundation and contributors https://js.foundation
dev,multer,MIT,Copyright 2014 Hage Yaapa
Expand Down
1 change: 1 addition & 0 deletions integration-tests/esbuild/basic-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const tracer = require('../../').init() // dd-trace
const assert = require('assert')
const express = require('express')
const http = require('http')
require('knex') // has dead code paths for multiple instrumented packages

const app = express()
const PORT = 31415
Expand Down
13 changes: 12 additions & 1 deletion integration-tests/esbuild/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,18 @@ esbuild.build({
outfile: 'out.js',
plugins: [ddPlugin],
platform: 'node',
target: ['node16']
target: ['node16'],
external: [
// dead code paths introduced by knex
'pg',
'mysql2',
'better-sqlite3',
'sqlite3',
'mysql',
'oracledb',
'pg-query-stream',
'tedious'
]
}).catch((err) => {
console.error(err) // eslint-disable-line no-console
process.exit(1)
Expand Down
3 changes: 2 additions & 1 deletion integration-tests/esbuild/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"license": "ISC",
"dependencies": {
"esbuild": "0.16.12",
"express": "^4.16.2"
"express": "^4.16.2",
"knex": "^2.4.2"
}
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@
"graphql": "0.13.2",
"int64-buffer": "^0.1.9",
"jszip": "^3.5.0",
"knex": "^2.4.2",
"mkdirp": "^0.5.1",
"mocha": "8",
"msgpack-lite": "^0.1.26",
Expand Down
14 changes: 13 additions & 1 deletion packages/datadog-esbuild/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,19 @@ module.exports.setup = function (build) {

if (args.namespace === 'file' && packagesOfInterest.has(packageName)) {
// The file namespace is used when requiring files from disk in userland
const pathToPackageJson = require.resolve(`${packageName}/package.json`, { paths: [ args.resolveDir ] })

let pathToPackageJson
try {
pathToPackageJson = require.resolve(`${packageName}/package.json`, { paths: [ args.resolveDir ] })
} catch (err) {
if (err.code === 'MODULE_NOT_FOUND') {
console.warn(`Unable to open "${packageName}/package.json". Is the "${packageName}" package dead code?`)
return
} else {
throw err
}
}

const pkg = require(pathToPackageJson)

if (DEBUG) {
Expand Down

0 comments on commit 5082455

Please sign in to comment.