Skip to content

Conversation

drjoeycadieux
Copy link

Resolves the TODO comment in resolveHttpServer by replacing @ts-expect-error with a proper type cast and explanatory comment.

When allowHTTP1: true is set, the HTTP/2 server can handle HTTP/1.1 requests, making the Connect app compatible as a request listener since:

  • Http2ServerRequest extends IncomingMessage
  • Http2ServerResponse extends ServerResponse
  • The server operates in HTTP/1.1 compatibility mode

This change improves code clarity and maintainability by removing the ambiguous TODO and explaining the type compatibility.

Description

Resolves the TODO comment in resolveHttpServer by replacing @ts-expect-error
with a proper type cast and explanatory comment.

When allowHTTP1: true is set, the HTTP/2 server can handle HTTP/1.1 requests,
making the Connect app compatible as a request listener since:
- Http2ServerRequest extends IncomingMessage
- Http2ServerResponse extends ServerResponse
- The server operates in HTTP/1.1 compatibility mode

This change improves code clarity and maintainability by removing the
ambiguous TODO and explaining the type compatibility.
@sapphi-red
Copy link
Member

  • Http2ServerRequest extends IncomingMessage
  • Http2ServerResponse extends ServerResponse

Where is this written / ensured? The type definition or the actual implementation does not extend them.
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/3f73ff61db465c1ad8cc5559789be7afec21bdc1/types/node/http2.d.ts#L1610
https://github.com/nodejs/node/blob/daf0a44669992ea2dff7f4a5b14e6f9088ce4399/lib/internal/http2/compat.js#L311

@sapphi-red sapphi-red changed the title fix(http): clarify HTTP/2 server compatibility with Connect app chore(http): clarify HTTP/2 server compatibility with Connect app Oct 15, 2025
@sapphi-red sapphi-red changed the title chore(http): clarify HTTP/2 server compatibility with Connect app chore: clarify HTTP/2 server compatibility with Connect app Oct 15, 2025
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.

2 participants