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

Rationale for moving @types/... to dependencies? #2580

Open
1 of 2 tasks
astellingwerf opened this issue Dec 4, 2024 · 2 comments
Open
1 of 2 tasks

Rationale for moving @types/... to dependencies? #2580

astellingwerf opened this issue Dec 4, 2024 · 2 comments

Comments

@astellingwerf
Copy link

I'd like to understand why in #468 @obecny moved several @types/... dependencies from devDependencies (where they typically belong) to production dependencies. It means that I ship a bunch of extra libraries, while they are not used.
Why was this change made, and would it make sense to move them back?

  • This only affects the JavaScript OpenTelemetry library
  • This may affect other libraries, but I would like to get opinions here first
@pichlermarc
Copy link
Member

@blumamir has created a really good write-up of this. The reason why some of these are in dependencies is because they're in some way shape or form part of the public API of that package. When they're part of the public API and the package is not included as a dependency, it may break the typescript build for the user.

The write-up above also notes that wherever possible we should avoid this in instrumentations. We've since then removed these types from the public API wherever possible and replaced it with something else. AFAIK, there's a few packages left though that we have not gotten to or that don't have owners (see this list) at the moment.

@obecny
Copy link
Member

obecny commented Dec 4, 2024

@astellingwerf
Well that was a long time ago but from what I remember we had some problems and it appeared that the typescripts weren't compiled when we had them in dev packages. Those types were still needed as they were exported. We discussed this on some meeting and that was the approach we took. Please feel free to propose whatever solutions works better, as long as it compiles I think it will be good to change just maybe be sure to get "yes" from current maintainers before you start changing this to avoid any confussions or unnecessary work, thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants