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

Express, HTTP/2 and Node.js Core #27

Closed
jasnell opened this issue Aug 1, 2016 · 6 comments
Closed

Express, HTTP/2 and Node.js Core #27

jasnell opened this issue Aug 1, 2016 · 6 comments

Comments

@jasnell
Copy link

jasnell commented Aug 1, 2016

Hello @expressjs/express-tc!

I am currently heading up an effort to explore whether and how to implement HTTP/2 support directly in Node.js core. The question of If we should do this is still open. To help answer that question, as well as determine the best course forward should we decide to move ahead, I would love to get input from each of you with regards to what would be the best approach from the Express point of view.

Refs: nodejs/CTC#6

@hacksparrow
Copy link
Member

It would be great to have fully compliant native HTTP/2 support in Node.js.

My understanding is that Express does not have to deal with the request and response objects at the HTTP protocol level, so as long as the request and response objects are HTTP/2-ready, Express should work just fine.

We might need to add additional methods and properties, though. I haven't though much about them yet.

@jasnell
Copy link
Author

jasnell commented Aug 1, 2016

One of the key questions with moving forward on this is whether or not the HTTP/2 implementation should seek to maintain API compatibility with the existing HTTP module API. For the overwhelming majority of it, it should be fairly easy to ensure API compatibility but there are a few areas where it simply is not practical -- push streams, for instance, and the fact that there's no such thing as a status message in HTTP/2. For the most part, the API would be nearly identical but, as we all know, the devil is in the details and small API changes could lead to significant issues. So I guess the main thing to figure out would be which parts of the API are ok to change, which parts are not, and what is the best process for staging out those changes in the code.

Another point is that while implementing the HTTP/2 support, we could take the opportunity to make necessary improvements on the existing API design in order to make it more efficient, more manageable, and easier to evolve over time.... but, I'd rather not break everything in the process.

@jonathanong
Copy link
Member

i don't mind if it's in node core or not, only that it's part of the node foundation and there's only one. i just don't want to choose between supporting multiple http2 servers.

@tunniclm
Copy link

tunniclm commented Aug 10, 2016

I did some investigation while attempting to get the node-http2 module working properly with Express. This module managed to keep a very similar API to the core https module. The main reason Express wouldn't work properly with the module was due to its different inheritance chain for request and response objects compared to the core http/https.

Express messes with the __proto__ of the request and response in order to inject its own api extension methods into the inheritance chain. I saw some commentary indicating this approach was taken (despite being fairly brittle) for performance reasons. I don't have any results to back that up though. The problematic part is that Express assumes the request/response will have a particular __proto__ and replaces it with its own that, in turn defers to the assumed original. In the case of node-http2 this is an incorrect assumption and it breaks.

The other concern that has been raised is less specific to HTTP/2, but interesting nonetheless: that is that there have been naming clashes between new versions of the http/https request and response objects and the additional Express methods. I think this was already discussed somewhere within the http working group, so perhaps a solution already exists for this potential issue.

@jasnell
Copy link
Author

jasnell commented Aug 11, 2016

Beginnings of the nodejs proposal: nodejs/node-eps#38

@mastermatt
Copy link

HTTP2 has been in Node for several years now. Closing this issue from lack of activity and any further HTTP2 discussions would probably belong in their own issue anyway.

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

No branches or pull requests

5 participants