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

fix(koa)!: use generic config hook types and move dep to dev #2303

Merged
merged 3 commits into from
Jun 27, 2024

Conversation

blumamir
Copy link
Member

fixes #2228

This PR aims to remove the instrumentation package dependency on the koa types packages, by making the relevant types generic and defaulting to any. That allows users who choose to implement the hook, to either use type any, or install the relevant types packages and populate the generic parameters.

  • moved koa types packages to dev dependency instead of regular dependency
  • documented how to use in README
  • updated tests to verify it compiles correctly when used in recommended way
  • compiled the new code and verified that d.ts files transitively imported from index.d.ts do not import types from @types/koa, @types/koa__router or any other dev dependency

Breaking Change

  • this PR makes it so that KoaContext is no longer exported as public type from the package
  • some hook info properties type will change to any by default, which should not break anyone I believe, but will no not verified this variable typing as before.

Prior Art

defaults to any and allow to use generics to type correctly:

vendors part of the type as default, but allows to specify the full type when desired:

@blumamir blumamir requested a review from a team June 26, 2024 22:43
@github-actions github-actions bot added pkg:instrumentation-koa pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found. labels Jun 26, 2024
@blumamir blumamir changed the title fix!(koa): use feneric config hook types and move dep to dev fix(koa)!: use feneric config hook types and move dep to dev Jun 26, 2024
@blumamir blumamir changed the title fix(koa)!: use feneric config hook types and move dep to dev fix(koa)!: use generic config hook types and move dep to dev Jun 26, 2024
Copy link

codecov bot commented Jun 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.30%. Comparing base (dfb2dff) to head (faa5cdd).
Report is 174 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2303      +/-   ##
==========================================
- Coverage   90.97%   90.30%   -0.68%     
==========================================
  Files         146      147       +1     
  Lines        7492     7263     -229     
  Branches     1502     1509       +7     
==========================================
- Hits         6816     6559     -257     
- Misses        676      704      +28     
Files Coverage Δ
...lemetry-instrumentation-koa/src/instrumentation.ts 95.29% <100.00%> (-0.11%) ⬇️
...elemetry-instrumentation-koa/src/internal-types.ts 100.00% <ø> (ø)
...ode/opentelemetry-instrumentation-koa/src/types.ts 100.00% <ø> (ø)
...ode/opentelemetry-instrumentation-koa/src/utils.ts 100.00% <100.00%> (ø)

... and 58 files with indirect coverage changes

@pichlermarc pichlermarc added the bug Something isn't working label Jun 27, 2024
Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks 👍
Another instrumentation that had the instrumented package's types part of their public interface crossed of the list 🙂

@pichlermarc pichlermarc merged commit d9d558f into open-telemetry:main Jun 27, 2024
13 checks passed
@dyladan dyladan mentioned this pull request Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:instrumentation-koa pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found.
Projects
None yet
2 participants