appsignal / appsignal-nodejs

🟩 AppSignal for Node.js
https://www.appsignal.com/nodejs/
MIT License
28 stars 9 forks source link

Fix Yarn and pnpm support by declaring required transitive dependencies #1037

Closed unflxw closed 4 months ago

unflxw commented 4 months ago

We mostly test the AppSignal for Node.js integration with npm, which lets you do whatever in terms of dependency use.

When @appsignal/nodejs attempts to require("@opentelemetry/sdk-metrics"), which is not a declared dependency of @appsignal/nodejs, but it is a transitive dependency (via @opentelemetry/sdk-node), npm complies by picking any version of @opentelemetry/sdk-node it can find in the dependency tree.

This behaviour can be problematic. Less chaos-friendly package managers, such as Yarn and pnpm, do not allow requiring transitive dependencies, and throw an error instead. Unfortunately, because we do the above, that means that AppSignal is not compatible with Yarn and pnpm.

Fortunately, there's no reason why we have to require undeclared transitive dependencies. We should declare @opentelemetry/sdk-metrics (and any other undeclared-but-required dependencies) as dependencies or peer dependencies of our package.

Related docs change: https://github.com/appsignal/appsignal-docs/pull/1860