-
Notifications
You must be signed in to change notification settings - Fork 513
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
@opentelemetry/instrumentation-graphql
Version 0.27.1
Missing @types/graphql
dependency
#802
Comments
Please note that Other instrumentations may have similar issues. As far as I remember the easiest workaround is to set |
As a long-term solution that doesn't require our users to skip lib check we could hand-roll our own types (or copy official ones) |
Isn't copying the types more error prone than simply making That being said I'm not sure what the best solution is but definitely the workaround suggested is not a great answer. Since the graphql instrumentation is one of the requirements for regular node auto-instrumentation package I do hope a fix for this gets bumped up since it's affecting users who have no interest in the graphql instrumentation itself. |
yes, but taking
It may not be the long-term solution, but that's why its called a work around :) It seems to me that there are only two workable solutions:
|
@dyladan thanks for the prompt response and just be clear I appreciate what's going on here and I didn't intend to cast a negative spin on the conversation. I understand the workaround is temporary but I should have been more articulate and said that it makes me nervous to introduce impactful changes into a libraries typescript configuration since A) they can are poorly understood and B) they can easily be overlooked in the future. Anyway, I'm glad you have a clear visibility on a long term solution because I can see this compilation error biting anyone who pulls down |
FYI installing the deprecated package |
@asafpelegcodes |
Actually the version issue was caused because we had |
We had I was assuming that version Maybe incremental adding dependencies causes also problems (e.g. install first |
I had forgotten that detail
In general, I think this deduplication is usually pretty good, but it might be possible to end up with a scenario like this:
I'm not sure if this would cause problems or not though even if it happened. |
The problem we experienced was caused because the latest |
@nata7che Can you remember what you selected in your package.json? Did you use npm, yarn or something else? |
@Flarna yep, it was |
Yarn actually checks the |
So what is the recommended workaround at the moment? |
Current workaround is probably to do The long-term solution will probably be to vendor the types in-package. |
@shaicantor on my project we decided to go with installing |
What version of OpenTelemetry are you using?
1.0.4
What version of Node are you using?
v16.11.0
Please provide the code you used to setup the OpenTelemetry SDK
What did you do?
In a fresh folder
Add the build script to your
package.json
Attempt to compile the typescript
What did you expect to see?
I expected the file to compile successfully
What did you see instead?
The file failed to compile because the graphql dependency was not installed
Additional context
In Version
0.26.1
it contains the dependency"@types/graphql": "14.5.0"
which has somehow been dropped in the latest versionThe text was updated successfully, but these errors were encountered: