Skip to content

Commit 65e18d0

Browse files
uurienuurien
authored
Improve support for esbuild + esm with DD_BUILD_ESM env var (#5888)
Fix __dirname and __filename in esbuild + esm by adding globals to the banner option.
1 parent 7e544f6 commit 65e18d0

11 files changed

+303
-0
lines changed

‎integration-tests/esbuild/build-and-run.esm-patched-const-banner.mjs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#!/usr/bin/env node
2+
/* eslint-disable no-console */
3+
4+
import esbuild from 'esbuild'
5+
import { spawnSync } from 'child_process'
6+
import commonConfig from './build.esm.common-config.js'
7+
8+
await esbuild.build({
9+
...commonConfig,
10+
banner: {
11+
js: `import { createRequire } from 'module';
12+
import { fileURLToPath } from 'url';
13+
import { dirname } from 'path';
14+
const require = createRequire(import.meta.url);
15+
const __filename = fileURLToPath(import.meta.url);
16+
const __dirname = dirname(__filename);`
17+
}
18+
})
19+
20+
const { status, stdout, stderr } = spawnSync('node', ['out.mjs'])
21+
if (stdout.length) {
22+
console.log(stdout.toString())
23+
}
24+
if (stderr.length) {
25+
console.error(stderr.toString())
26+
}
27+
if (status) {
28+
throw new Error('generated script failed to run')
29+
}
30+
console.log('ok')

‎integration-tests/esbuild/build-and-run.esm-patched-global-banner.mjs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#!/usr/bin/env node
2+
/* eslint-disable no-console */
3+
4+
import esbuild from 'esbuild'
5+
import { spawnSync } from 'child_process'
6+
import commonConfig from './build.esm.common-config.js'
7+
8+
await esbuild.build({
9+
...commonConfig,
10+
banner: {
11+
js: `import { createRequire } from 'module';
12+
import { fileURLToPath } from 'url';
13+
import { dirname } from 'path';
14+
globalThis.require ??= createRequire(import.meta.url);
15+
globalThis.__filename ??= fileURLToPath(import.meta.url);
16+
globalThis.__dirname ??= dirname(globalThis.__filename);`
17+
}
18+
})
19+
20+
const { status, stdout, stderr } = spawnSync('node', ['out.mjs'])
21+
if (stdout.length) {
22+
console.log(stdout.toString())
23+
}
24+
if (stderr.length) {
25+
console.error(stderr.toString())
26+
}
27+
if (status) {
28+
throw new Error('generated script failed to run')
29+
}
30+
console.log('ok')

‎integration-tests/esbuild/build-and-run.esm-relying-in-extension.mjs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#!/usr/bin/env node
2+
/* eslint-disable no-console */
3+
4+
import esbuild from 'esbuild'
5+
import { spawnSync } from 'child_process'
6+
import commonConfig from './build.esm.common-config.js'
7+
8+
await esbuild.build({
9+
...commonConfig,
10+
format: undefined
11+
})
12+
13+
const { status, stdout, stderr } = spawnSync('node', ['out.mjs'])
14+
if (stdout.length) {
15+
console.log(stdout.toString())
16+
}
17+
if (stderr.length) {
18+
console.error(stderr.toString())
19+
}
20+
if (status) {
21+
throw new Error('generated script failed to run')
22+
}
23+
console.log('ok')

‎integration-tests/esbuild/build-and-run.esm-relying-in-format.mjs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#!/usr/bin/env node
2+
/* eslint-disable no-console */
3+
4+
import esbuild from 'esbuild'
5+
import { spawnSync } from 'child_process'
6+
import { renameSync } from 'fs'
7+
import commonConfig from './build.esm.common-config.js'
8+
9+
await esbuild.build({
10+
...commonConfig,
11+
outfile: 'out.js'
12+
})
13+
14+
// to force being executed as module
15+
renameSync('./out.js', './out.mjs')
16+
17+
const { status, stdout, stderr } = spawnSync('node', ['out.mjs'])
18+
if (stdout.length) {
19+
console.log(stdout.toString())
20+
}
21+
if (stderr.length) {
22+
console.error(stderr.toString())
23+
}
24+
if (status) {
25+
throw new Error('generated script failed to run')
26+
}
27+
console.log('ok')

‎integration-tests/esbuild/build-and-run.esm-relying-in-out-extension.mjs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#!/usr/bin/env node
2+
/* eslint-disable no-console */
3+
4+
import esbuild from 'esbuild'
5+
import { spawnSync } from 'child_process'
6+
import commonConfig from './build.esm.common-config.js'
7+
8+
// output => basic-test.mjs
9+
await esbuild.build({
10+
...commonConfig,
11+
outfile: undefined,
12+
format: undefined,
13+
outdir: './',
14+
outExtension: { '.js': '.mjs' }
15+
})
16+
17+
const { status, stdout, stderr } = spawnSync('node', ['basic-test.mjs'])
18+
if (stdout.length) {
19+
console.log(stdout.toString())
20+
}
21+
if (stderr.length) {
22+
console.error(stderr.toString())
23+
}
24+
if (status) {
25+
throw new Error('generated script failed to run')
26+
}
27+
console.log('ok')

‎integration-tests/esbuild/build-and-run.esm-unrelated-js-banner.mjs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#!/usr/bin/env node
2+
/* eslint-disable no-console */
3+
4+
import esbuild from 'esbuild'
5+
import { spawnSync } from 'child_process'
6+
import commonConfig from './build.esm.common-config.js'
7+
8+
await esbuild.build({
9+
...commonConfig,
10+
banner: {
11+
js: '/* js test */'
12+
}
13+
})
14+
15+
const { status, stdout, stderr } = spawnSync('node', ['out.mjs'])
16+
if (stdout.length) {
17+
console.log(stdout.toString())
18+
}
19+
if (stderr.length) {
20+
console.error(stderr.toString())
21+
}
22+
if (status) {
23+
throw new Error('generated script failed to run')
24+
}
25+
console.log('ok')

‎integration-tests/esbuild/build.esm.common-config.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
const ddPlugin = require('../../esbuild')
2+
module.exports = {
3+
format: 'esm',
4+
entryPoints: ['basic-test.js'],
5+
bundle: true,
6+
outfile: 'out.mjs',
7+
plugins: [ddPlugin],
8+
platform: 'node',
9+
target: ['node18'],
10+
external: [
11+
// dead code paths introduced by knex
12+
'pg',
13+
'mysql2',
14+
'better-sqlite3',
15+
'sqlite3',
16+
'mysql',
17+
'oracledb',
18+
'pg-query-stream',
19+
'tedious'
20+
]
21+
}

‎integration-tests/esbuild/build.esm.mjs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#!/usr/bin/env node
2+
3+
import esbuild from 'esbuild'
4+
5+
import commonConfig from './build.esm.common-config.js'
6+
7+
await esbuild.build(commonConfig)

‎integration-tests/esbuild/index.spec.js

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
const chproc = require('child_process')
88
const path = require('path')
99
const fs = require('fs')
10+
const { assert } = require('chai')
1011

1112
const TEST_DIR = path.join(__dirname, '.')
1213
process.chdir(TEST_DIR)
@@ -75,5 +76,93 @@ esbuildVersions.forEach((version) => {
7576
timeout: 1000 * 30
7677
})
7778
})
79+
80+
describe('ESM', () => {
81+
afterEach(() => {
82+
fs.rmSync('./out.mjs', { force: true })
83+
fs.rmSync('./out.js', { force: true })
84+
fs.rmSync('./basic-test.mjs', { force: true })
85+
})
86+
87+
it('works', () => {
88+
console.log('npm run build:esm')
89+
chproc.execSync('npm run build:esm')
90+
console.log('npm run built:esm')
91+
chproc.execSync('npm run built:esm', {
92+
timeout: 1000 * 30
93+
})
94+
})
95+
96+
it('should not override existing js banner', () => {
97+
const command = 'node ./build-and-run.esm-unrelated-js-banner.mjs'
98+
console.log(command)
99+
chproc.execSync(command, {
100+
timeout: 1000 * 30
101+
})
102+
103+
const builtFile = fs.readFileSync('./out.mjs').toString()
104+
assert.include(builtFile, '/* js test */')
105+
})
106+
107+
it('should contain the definitions when esm is inferred from outfile', () => {
108+
const command = 'node ./build-and-run.esm-relying-in-extension.mjs'
109+
console.log(command)
110+
chproc.execSync(command, {
111+
timeout: 1000 * 30
112+
})
113+
114+
const builtFile = fs.readFileSync('./out.mjs').toString()
115+
assert.include(builtFile, 'globalThis.__filename ??= $dd_fileURLToPath(import.meta.url);')
116+
})
117+
118+
it('should contain the definitions when esm is inferred from format', () => {
119+
const command = 'node ./build-and-run.esm-relying-in-format.mjs'
120+
console.log(command)
121+
chproc.execSync(command, {
122+
timeout: 1000 * 30
123+
})
124+
125+
const builtFile = fs.readFileSync('./out.mjs').toString()
126+
assert.include(builtFile, 'globalThis.__filename ??= $dd_fileURLToPath(import.meta.url);')
127+
})
128+
129+
it('should contain the definitions when format is inferred from out extension', () => {
130+
const command = 'node ./build-and-run.esm-relying-in-out-extension.mjs'
131+
console.log(command)
132+
chproc.execSync(command, {
133+
timeout: 1000 * 30
134+
})
135+
136+
const builtFile = fs.readFileSync('./basic-test.mjs').toString()
137+
assert.include(builtFile, 'globalThis.__filename ??= $dd_fileURLToPath(import.meta.url);')
138+
})
139+
140+
it('should not contain the definitions when no esm is specified', () => {
141+
const command = 'node ./build.js'
142+
console.log(command)
143+
chproc.execSync(command, {
144+
timeout: 1000 * 30
145+
})
146+
147+
const builtFile = fs.readFileSync('./out.js').toString()
148+
assert.notInclude(builtFile, 'globalThis.__filename ??= $dd_fileURLToPath(import.meta.url);')
149+
})
150+
151+
it('should not crash when it is already patched using global', () => {
152+
const command = 'node ./build-and-run.esm-patched-global-banner.mjs'
153+
console.log(command)
154+
chproc.execSync(command, {
155+
timeout: 1000 * 30
156+
})
157+
})
158+
159+
it('should not crash when it is already patched using const', () => {
160+
const command = 'node ./build-and-run.esm-patched-const-banner.mjs'
161+
console.log(command)
162+
chproc.execSync(command, {
163+
timeout: 1000 * 30
164+
})
165+
})
166+
})
78167
})
79168
})

‎integration-tests/esbuild/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
"main": "app.js",
77
"scripts": {
88
"build": "DD_TRACE_DEBUG=true node ./build.js",
9+
"build:esm": "DD_TRACE_DEBUG=true node ./build.esm.mjs",
910
"built": "DD_TRACE_DEBUG=true node ./out.js",
11+
"built:esm": "DD_TRACE_DEBUG=true node ./out.mjs",
1012
"raw": "DD_TRACE_DEBUG=true node ./app.js",
1113
"link": "pushd ../.. && yarn link && popd && yarn link dd-trace",
1214
"request": "curl http://localhost:3000 | jq"

‎packages/datadog-esbuild/index.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,30 @@ for (const pkg of INSTRUMENTED) {
5353

5454
module.exports.name = 'datadog-esbuild'
5555

56+
function isESMBuild (build) {
57+
// check toLowerCase? to be safe if unexpected object is there instead of a string
58+
const format = build.initialOptions.format?.toLowerCase?.()
59+
const outputFile = build.initialOptions.outfile?.toLowerCase?.()
60+
const outExtension = build.initialOptions.outExtension?.['.js']
61+
return format === 'esm' || outputFile?.endsWith('.mjs') || outExtension === '.mjs'
62+
}
63+
5664
module.exports.setup = function (build) {
5765
const externalModules = new Set(build.initialOptions.external || [])
66+
if (isESMBuild(build)) {
67+
build.initialOptions.banner ??= {}
68+
build.initialOptions.banner.js ??= ''
69+
if (!build.initialOptions.banner.js.includes('import { createRequire as $dd_createRequire } from \'module\'')) {
70+
build.initialOptions.banner.js = `import { createRequire as $dd_createRequire } from 'module';
71+
import { fileURLToPath as $dd_fileURLToPath } from 'url';
72+
import { dirname as $dd_dirname } from 'path';
73+
globalThis.require ??= $dd_createRequire(import.meta.url);
74+
globalThis.__filename ??= $dd_fileURLToPath(import.meta.url);
75+
globalThis.__dirname ??= $dd_dirname(globalThis.__filename);
76+
${build.initialOptions.banner.js}`
77+
}
78+
}
79+
5880
build.onResolve({ filter: /.*/ }, args => {
5981
if (externalModules.has(args.path)) {
6082
// Internal Node.js packages will still be instrumented via require()

0 commit comments

Comments
 (0)