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

[bug]: Dev server and upward-js do not support HTTP2 #1271

Closed
4 of 6 tasks
zetlen opened this issue May 24, 2019 · 1 comment
Closed
4 of 6 tasks

[bug]: Dev server and upward-js do not support HTTP2 #1271

zetlen opened this issue May 24, 2019 · 1 comment
Labels
bug Something isn't working help wanted Eligible for community contribution.

Comments

@zetlen
Copy link
Contributor

zetlen commented May 24, 2019

Describe the bug
Our local server environments don't use HTTP2. This is a serious performance problem.

We use webpack-dev-server as our local development server, and upward-js as our local production staging server. Both of those use Express as their HTTP handling framework, and Express still does not support HTTP2, after promising it for years.

In this issue, Express maintainers say that Express is HTTP version-agnostic, but the maintainers of webpack-dev-server in this issue disagree; since Express claims compatibility only with the node-spdy module, and that module is basically unmaintained. It's a circle of pointing fingers and it suggests that we do not know when HTTP2 support will arrive as part of the natural update cycle.

To Reproduce
Steps to reproduce the behavior:

  1. Check out the develop branch.
  2. Run yarn install && yarn build && yarn watch:venia
  3. Copy the dev server URL to your clipboard.
  4. Open a browser tab. While it is still blank, open the developer tools and go to the Network tab.
  5. Paste and load the dev server URL.
  6. Look at the Protocol column in the network request table.

Expected behavior
-h2 should be the Protocol

  • Requests should load in a massively parallel way, instead of the HTTP 1 limit of six at a time
  • Server push should be possible, though not implemented yet

Screenshots

Dev server loading with HTTP1
image

Dev server loading with HTTP2 (I hacked it just to make the screenshot)
image

Additional context
HTTP2 is very important for us to get right. Not only for the speed and concurrency it offers and the server push capability that enables outstanding prefetch, but also for less political, more marketing-related reasons:

  • It's a default expectation for a modern offering
  • It's part of the PWA best practices guide
  • Lighthouse rewards it
  • Competitors have it

The good news is that HTTP2 is working in all of our production environments.

So this is a high severity issue, but not a high priority one. We need to be "native HTTP2", and right now, the only reason that our production systems are issuing HTTP2 responses is that the deployment environments are wrapping our server in their own optimization.

Possible solutions
First of all, we should be using native NodeJS http2, which has existed since Node 8. We shouldn't be using the spdy module, as express continues to do. Some ideas:

  • Switch to koa, which would require a bunch of refactoring of middlewares but not a huge architectural change
  • Switch to micro, which would require a bigger architectural change to the dev and prod servers, but be the most aggressively fast and leading-edge
  • Stop using a web server library abstraction at all and use Node's increasingly-friendly HTTP2 library directly

All of these would require, at the very least, that:

  • we move off webpack-dev-server and re-architect our hot reloading environment
  • we reimplement upward-js to use HTTP2 (and extend UPWARD with http2-only concepts to make it the best practices enforcing system it needs to be.
  • we do the due diligence to ensure that we're taking advantage of HTTP2 in all our recommended deployment targets

Please let us know what packages this bug is in regards to:

  • venia-concept
  • pwa-buildpack
  • peregrine
  • pwa-devdocs
  • upward-js
  • upward-spec
@zetlen zetlen added the bug Something isn't working label May 24, 2019
@awilcoxa awilcoxa added the MM19UK label Jun 6, 2019
@supernova-at supernova-at added help wanted Eligible for community contribution. and removed MM19UK labels Jun 24, 2019
@sidolov sidolov added this to Ready for Grooming in Community Issue Tracking Aug 8, 2019
@awilcoxa awilcoxa added this to the UPWARD Improvements Phase 2 milestone Sep 16, 2019
@awilcoxa
Copy link

Closing based on grooming review.

Community Issue Tracking automation moved this from Ready for Grooming to Done Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Eligible for community contribution.
Projects
Development

No branches or pull requests

3 participants