denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
93.33k stars 5.17k forks source link

`node:http2` doesn't export `Http2ServerRequest` and `Http2ServerResponse` #23326

Open bartlomieju opened 2 months ago

bartlomieju commented 2 months ago

Discussed in https://github.com/denoland/deno/discussions/23324

Originally posted by **OnielN14** April 11, 2024 In the v1.42.2 I see latest compat update for node http2 is released. So I just try to run hono + @hono/node-server ```ts import { serve } from "npm:@hono/node-server" import { Hono } from "npm:hono" const app = new Hono() app.get("/", (c) => c.text("Hello from server")) serve({ fetch: app.fetch, port: 3000 }, () => { console.log("Running on 3000") }) ``` and it seems `Http2ServerRequest` is not available as named export. here is the error message ``` error: Uncaught SyntaxError: The requested module 'http2' does not provide an export named 'Http2ServerRequest' at (file:///C:/Users/OnielN14/AppData/Local/deno/npm/registry.npmjs.org/@hono/node-server/1.9.1/dist/index.mjs:5:10) ``` am I missing something regarding node compatiblity? Thanks
bartlomieju commented 2 months ago

No particular reason, it's a bug.

clemg commented 2 months ago

Just exporting these two classes (here and here) won't make it work. The response here will be empty

bartlomieju commented 2 months ago

That's correct, @satyarohith is working on improving node:http2 module.

jmj0502 commented 1 month ago

@bartlomieju Is this issue still open? I mean can I work on this? Is not entirely clear based on your last comment.

If so, I would like to give it a stab c:

satyarohith commented 1 month ago

@bartlomieju Is this issue still open? I mean can I work on this? Is not entirely clear based on your last comment.

If so, I would like to give it a stab c:

Hi, @jmj0502. Please go ahead. Note https://github.com/denoland/deno/issues/23326#issuecomment-2056651490

jmj0502 commented 1 month ago

@satyarohith cool!

Quick question? Should I work on the implementation describe on the initial consideration of the issue? or are there other changes to be considered for the issue in question? 🤔

satyarohith commented 1 month ago

@satyarohith cool!

Quick question? Should I work on the implementation describe on the initial consideration of the issue? or are there other changes to be considered for the issue in question? 🤔

A good target is to get the example from description working with Deno. Exporting the unavailable methods isn't sufficient to get it working as mentioned here. I'm not entirely sure what exactly is required to get the example to work with Deno. We need to explore why the response is empty and what needs to be done. The scope of it has increased after https://github.com/denoland/deno/issues/23326#issuecomment-2056651490 and I don't think it's a good first issue anymore. But I'm happy to help (You can send me Discord DM) if you're interested to dig deeper. :)

jmj0502 commented 1 month ago

A good target is to get the example from https://github.com/denoland/deno/issues/23326#issue-2238489014 working with Deno. Exporting the unavailable methods isn't sufficient to get it working as mentioned https://github.com/denoland/deno/issues/23326#issuecomment-2056651490. I'm not entirely sure what exactly is required to get the example to work with Deno. We need to explore why the response is empty and what needs to be done. The scope of it has increased after https://github.com/denoland/deno/issues/23326#issuecomment-2056651490 and I don't think it's a good first issue anymore. But I'm happy to help (You can send me Discord DM) if you're interested to dig deeper. :)

@satyarohith Cool! I'm all up for it! Many thanks for offering your help, again I don't really know the codebase in depth, but I'll do my best to fix the issue in question.

I'll start by building the project and replicating the issue in question, then I'll proceed to send you a discord DM to further explain my plan of action and start diving deeper into the issue. Feel free to assign the issue to me if you consider it appropriate 👍

EdamAme-x commented 3 weeks ago

I also encountered the same issue