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(http,ws): conn typings on internal utils #861

Closed
wants to merge 1 commit into from

Conversation

wperron
Copy link
Contributor

@wperron wperron commented Apr 16, 2021

Update the return type from Deno.Conn to Deno.Conn<Deno.NetAddr> on
two of the internal functions used to mock conn objects. The issue was
introduced in core at 9c7c9a35.

In the case of http/_mock_conn.ts@mockConn I also had to add an
explicit type hint on the return object because of the base parameter
that was causing the type checker to not like the transport property
on localAddr and remoteAddr.

Update the return type from `Deno.Conn` to `Deno.Conn<Deno.NetAddr>` on
two of the internal functions used to mock conn objects. The issue was
introduced in core at `9c7c9a35`.

In the case of `http/_mock_conn.ts@mockConn` I also had to add an
explicit type hint on the return object because of the `base` parameter
that was causing the type checker to not like the `transport` property
on `localAddr` and `remoteAddr`.
Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@lucacasonato lucacasonato left a comment

Choose a reason for hiding this comment

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

I don't want to land this. This being broken points to the upstream patch in CLI to be majorly breaking. It will break all old std versions. It should be reverted. cc @kitsonk

@wperron wperron closed this Apr 19, 2021
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.

None yet

3 participants