fix: promise connection typescript mismatched to underlying code

[ashleybartlett]

This fixes a couple of type errors compared to the underlying code.

This is likely a breaking change for some users as it would have required some working around to make it work before.

Should fix #3238 and #2667

Some future improvements that i'd like to see is the exported promise interfaces are converted to classes, to match the underlying code, but that felt like a much bigger change.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.45%. Comparing base (d48f143) to head (4e75c24). Report is 95 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3321   +/-   ##
=======================================
  Coverage   88.45%   88.45%           
=======================================
  Files          83       83           
  Lines       13067    13067           
  Branches     1393     1393           
=======================================
  Hits        11558    11558           
  Misses       1509     1509           

Flag	Coverage Δ
compression-0	88.45% <ø> (ø)
compression-1	88.45% <ø> (ø)
tls-0	87.84% <ø> (ø)
tls-1	88.21% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[wellwelwel]

Thanks, @ashleybartlett!

The inheritance from base connections after #3081 makes perfect sense, but I'll keep the #3273 issue as related due to an unexpected breaking change when using instanceof.

I'm planning to run some regression tests in your changes just to make sure we wouldn't imply an additional breaking change.

[ashleybartlett]

I think that breaking change makes a lot of sense, as it was done under the hood, but i'm not sure if it's something that can be fixed properly.

Given it wasn't breaking existing checks, I can revert that change for the base type if it's going to be a problem, and it can be revisited in a breaking change release.

[ashleybartlett]

I'll keep the #3273 issue as related due to an unexpected breaking change when using instanceof.

Revisiting this, I realised that instanceof would be a runtime check, not a typescript one. For this, I would ensure Typescript is accurately describing the Javascript code it's referencing.

[klingenm]

@wellwelwel What investigation is it you are looking for? Changing the types can not have any runtime effect and the PR currently only fixes a type error to match the actual implementation.

I was about to open a PR with the same fix, as I want to use the Promise api for connection handling but I want to access the base connection so I can use streaming responses. Now I have to typecast the promise connection type to the base connection type.

[wellwelwel]

Hey @ashleybartlett, the problem is not with this PR itself, but with issue #3273. Changing the types from Connection to Base...Connection or Core...Connection will force a breaking change for everyone who performs any checks or depends on the Connection type.

The mistake is probably in the class names in the base code and needs to be fixed there first, before going back to this PR.

I can't confirm if it would be so simple to resolve by just renaming the classes. It would be necessary to check the instances to address the problem at its root (that's the real investigation).