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

Move @types/ packages to devDependencies #1436

Closed

Conversation

lpsinger
Copy link

TypeScript type packages are normally dev-only dependencies. You generally only need types for development, not at run time.

Which problem is this PR solving?

  • Excessive size of node_modules directory when using these plugins.

Short description of the changes

  • Move all @types/ packages from dependencies to devDependencies.
  • Update GUIDELINES.md.

TypeScript type packages are normally dev-only dependencies. You
generally only need types for development, not at run time.
@Flarna
Copy link
Member

Flarna commented Mar 23, 2023

Have you ensured that these types are not used in any pubic API?
Otherwise consumers end up with typescript issues because of missing types.

The issue tracker holds a few such issues and there are a few PRs moving types from/to dev deps. e.a. #802

#1320 shows how to remove types dependencies in API surface to move them into dev deps without causing problems.

https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/GUIDELINES.md holds also some more details.

@lpsinger
Copy link
Author

Oh dear. This is more complicated than I at first realized.

@lpsinger lpsinger closed this Mar 24, 2023
@Flarna
Copy link
Member

Flarna commented Mar 24, 2023

@lpsinger I fully understand that you closed this PR.
But you are really welcome to contribute in this are. Likely not by a single big bang PR but maybe one by one.

@lpsinger
Copy link
Author

No worries, I understand.

I was looking at the AWS Distro for OpenTelemetry Lambda layer and I was surprised to find the type modules inside it.

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

Successfully merging this pull request may close these issues.

7 participants