Skip to content
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

esbuild bundling support external dependencies #3188

Closed
p-desaintchamas opened this issue May 25, 2023 · 1 comment · Fixed by #3215
Closed

esbuild bundling support external dependencies #3188

p-desaintchamas opened this issue May 25, 2023 · 1 comment · Fixed by #3215
Labels
bug Something isn't working bundler

Comments

@p-desaintchamas
Copy link

When bundling code with esbuild and using the dd-trace esbuild plugin, I run into an issue when resolving all dependencies, as some are specified as external for some useless dependencies I'm not using. Yet, dd-trace plugin still tries to resolve them.

Expected behaviour

external packages should not be resolved by the plugin

Actual behaviour

External packages are still resolved, causing issues when they are not added as dependencies

✘ [ERROR] Cannot find module 'tedious/package.json'
Require stack:
- /Users/**/node_modules/dd-trace/packages/datadog-esbuild/index.js
- /Users/**/node_modules/dd-trace/esbuild.js
- /Users/**/xbuild.config.js
- /Users/**/node_modules/@myscope/xbuild/dist/xbuild.js [plugin datadog-esbuild]

    node_modules/knex/lib/dialects/mssql/index.js:89:24:
      89 │     const tds = require('tedious');
         ╵                         ~~~~~~~~~

  This error came from the "onResolve" callback registered here:

    node_modules/dd-trace/packages/datadog-esbuild/index.js:38:8:
      38 │   build.onResolve({ filter: /.*/ }, args => {

Steps to reproduce

Sample project, with knex for example that has multiple external dependencies not useful, for example mysql if I'm using postgres, etc...

If I still try to add those dependencies, I run into another issue, which is that mysql2 is declared as instrumented, but the resolve is calling the package.json, which is not included in mysql2, causing the export to fail similarly. This would indicate that the list datadog-instrumentations/src/helpers/hooks.js is not entirely compatible.

✘ [ERROR] Package subpath './package.json' is not defined by "exports" in /Users/**/node_modules/mysql2/package.json [plugin datadog-esbuild]

    node_modules/dd-trace/packages/datadog-esbuild/index.js:45:40:
      45 │       const pathToPackageJson = require.resolve(`${packageName}/package.json`, { paths: [ args.resolveDir ] })

I think an easy fix would be to either:

  • filter resolved modules on being or not listed as external in esbuild build config.
  • add the ability for the user to specify a list of excluded modules when specifying the plugin, that would be complementary to the instrumented list provided by datadog.

Environment

  • Operation system: Macos
  • Node.js version: 16.19
  • Tracer version: 4.0
  • Agent version:
  • Relevant library versions:
@p-desaintchamas p-desaintchamas added the bug Something isn't working label May 25, 2023
@p-desaintchamas
Copy link
Author

p-desaintchamas commented May 25, 2023

For the mysql2 problem, I opened a merge request on their repo to allow the resolve to work as expected
sidorares/node-mysql2#2026

EDIT : PR has been merged and added to 3.3.3 release
I will test if it allow to fix today, but a support for external dependencies would still be the proper way to go, as this would enforce any user to include all libraries, and make the bundle size much more important

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working bundler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants