Add missing required transitive dependencies

appsignal / appsignal-nodejs

🟩 AppSignal for Node.js

https://www.appsignal.com/nodejs/

MIT License

28 stars 9 forks source link

Add missing required transitive dependencies #1050

Closed unflxw closed 4 months ago

unflxw commented 4 months ago

Declare the transitive dependencies that our code already imports as de facto dependencies in our package.json. Update the package-lock.json accordingly.

For @opentelemetry/instrumentation, which is only required for a type annotation, use import type instead.

Fixes #1037. [skip changeset]

backlog-helper[bot] commented 4 months ago

:heavy_check_mark: All good!

New issue guide | Backlog management | Rules | Feedback

unflxw commented 4 months ago

I have tested that this works with the latest Yarn version (in that it removes the error reported by the customer during the AppSignal build, allowing the build to succeed with a reasonable looking install report and ext/ folder -- I have not tested further)

backlog-helper[bot] commented 4 months ago

Hi @unflxw,

We've found new issues for this Pull Request. Please see the main comment on this issue for a list of all current warnings. This comment will not be updated to reflect resolved warnings.

This Pull Request does not include a changeset. Add a changeset if the change impacts users and should be included in the changelog upon release. Read more about changesets.
Ignore this rule by adding [skip changeset] to your Pull Request body. - (More info)

New issue guide | Backlog management | Rules | Feedback

backlog-helper[bot] commented 4 months ago

This Pull Request needs more reviews. @tombruijn - (More info)

This is a message from the daily scheduled checks.

New issue guide | Backlog management | Rules | Feedback

unflxw commented 4 months ago

@tombruijn I think you're right, I'll add a changeset of some sort, yes.

The thing is, I don't want us to proclaim in the docs again that we support Yarn or pnpm, though, like we used to do before this issue. Realistically, we're not doing proper testing with those tools.

But it would be nice to say we've made some improvements to the compatibility of our package with alternative package managers like Yarn or pnpm. Give them the same treatment we currently give to WSL: we don't explicitly support it, but if it works for you, that's great.