remix icon indicating copy to clipboard operation
remix copied to clipboard

feat(templates): add Link header for Early Hints, HTTP2 Server Push etc.

Open Plopix opened this issue 3 years ago • 16 comments

Hello,

Context

When using HTTP/2 we can optimize the first loading with HTTP/2 Server Push Link header. I had to implement it for a project and I wonder if it might cool to have it in Remix directly.

Probably not be the best place to put this code but at least this PR open the topic, and make it simple to test ;) Should we have that in Remix by default? or with a configuration?

What do you think?

Testing with HTTPS and HTTP2 locally

  • install Caddy Server
  • install mkcert and generate certificates

Then here a simple Caddyfile

my.custom.domain.com {
    tls ./provisioning/dev/certs/domains.pem ./provisioning/dev/certs/key.pem

    push
    
    @websockets {
        header Connection *Upgrade*
        header Upgrade websocket
    }
    # (3019 is custom, change for you)
    reverse_proxy @websockets localhost:3019
    
    # (3018 is custom, change for you)
    reverse_proxy 127.0.0.1:3018
}

Results

Screen Shot 2022-05-15 at 12 44 52 PM

Plopix avatar May 15 '22 19:05 Plopix

Hi @Plopix,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

remix-cla-bot[bot] avatar May 15 '22 19:05 remix-cla-bot[bot]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

remix-cla-bot[bot] avatar May 15 '22 19:05 remix-cla-bot[bot]

Hi @Plopix and thank you for opening this PR! Can you target the dev branch instead as you're changing code?

machour avatar May 16 '22 07:05 machour

Hello @machour, it's done!

Plopix avatar May 16 '22 15:05 Plopix

I like this idea, but I have a question, in daffy.org we import multiple copies of the same image in different sizes to support different pixel densities and for desktop and mobile, I then preload them using a link tag which supports imagesrcset and media for media queries.

Will this change send Link header to preload all of my images? Even the ones I don’t really need? Because right now an iPhone user for example only loader one of those images (mobile an 3x size) and not the rest, but Link header doesn’t support srcSet and media queries to conditionally load images.

sergiodxa avatar May 16 '22 17:05 sergiodxa

thanks @MichaelDeBoey much cleaner ;-) changed and pushed!

@sergiodxa the first piece of code will add the list of modules and imports (basically just the chunks).

see my screenshot

The second line answers "a bit more" your question:

responseHeaders.set("Link", (responseHeaders.has("Link") ? responseHeaders.get("Link") + "," : '') + http2PushLinksHeaders.map((link: string) => `<${link}>; rel=preload; as=script`).join(","));

If a Link header already exists then we add to it. In my screenshot, the tailwind CSS is using push because in my root.tsx I have:

export const headers: HeadersFunction = () => {
    return {
        Link: `<${tailwindStyles}>; rel=preload; as=style`,
    };
};

So to really answer your question the Link Tag is not related actually.

The preload value of the element's rel attribute lets you declare fetch requests in the HTML's , specifying resources that your page will need very soon, which you want to start loading early in the page lifecycle, before browsers' main rendering machinery kicks in. This ensures they are available earlier and are less likely to block the page's render, improving performance.

Here we the Link header is a HTTP/2 feature. That pushes the resources directly into the response, there is no roundtrip like with preload

Conclusion: it should not impact you ;-)

Plopix avatar May 17 '22 00:05 Plopix

I think we should remove the <link rel="modulepreload" href="/build/_shared/chunk-XXXX.js" /> from the rendered page. I don't really know the internal of Remix but that must be there: https://github.com/remix-run/remix/blob/main/packages/remix-react/components.tsx#L633 ping @MichaelDeBoey

Checking the header maybe, but there is something.

Plopix avatar May 18 '22 01:05 Plopix

btw @mjackson or @ryanflorence do you want to talk about that PR (like 10-15min) after the conf or tomorrow morning?

Plopix avatar May 25 '22 21:05 Plopix

I think we should remove the <link rel="modulepreload" href="/build/_shared/chunk-XXXX.js" /> from the rendered page. I don't really know the internal of Remix but that must be there: https://github.com/remix-run/remix/blob/main/packages/remix-react/components.tsx#L633 ping @MichaelDeBoey

Checking the header maybe, but there is something.

I think keeping them should not affect, right? The browser will already have the cached file so it will not request it again, and if for some reason the browser didn't downloaded the files from the header it has another opportunity to do it before actually needing the file.

sergiodxa avatar May 26 '22 03:05 sergiodxa

hey @sergiodxa no, I don't think about any side effect, and as it's in the entry.server developers will see it and can decide depending on their context if they want to keep it or not.

Plopix avatar May 30 '22 17:05 Plopix

hey @sergiodxa, no it should not indeed. And I've just added crossorigin=anonymous which is removing the annoying warnings that was triggered (thx @dhairyadwivedi)

Plopix avatar Jun 16 '22 18:06 Plopix

And by the way here, the HTTP 103 and Early Hints are working with this approach too right? so that's not just HTTP 2 Server Push, I am renaming the PR ;) any news on merging ?

Plopix avatar Jun 23 '22 07:06 Plopix

Hey! FYI this code no longer works in v1.10.0. I haven't found a solution yet unfortunately.

tom-sherman avatar Dec 31 '22 16:12 tom-sherman

Hey! FYI this code no longer works in v1.10.0. I haven't found a solution yet unfortunately.

😉 https://github.com/mcansh/mcan.sh/commit/934a7b1ab6145df2a6364df916a3d5c9f37e1fd8

that being said, maybe this is something we can do OOTB in remix itself? anyone want to open a proposal?

mcansh avatar Jan 01 '23 22:01 mcansh

@Plopix Could you please rebase your branch onto latest main & fix conflicts?

MichaelDeBoey avatar Jan 02 '23 01:01 MichaelDeBoey

Hey! FYI this code no longer works in v1.10.0. I haven't found a solution yet unfortunately.

😉 mcansh/mcan.sh@934a7b1

that being said, maybe this is something we can do OOTB in remix itself? anyone want to open a proposal?

I initially thought Remix could do this OOTB but now I think this should come in the template or be a function (I’m planning to add it to Remix Utils), the reason is because if the developer decides to remove the Scripts component this will push the JS files but nothing will use them.


Another thing is that this same technique can be used to know the linked assets and push them automatically, I was able to do it in my personal site to preload CSS and other assets https://github.com/sergiodxa/sergiodxa.com/blob/bf3ecf401b7e1406044d9480129cb5dd3063d0c0/app/entry.server.tsx#L53

sergiodxa avatar Jan 02 '23 23:01 sergiodxa

v1.14.0 removed the need to have entry files & we removed them from our templates as well.

@Plopix Could you rebase your PR onto latest dev & apply changes to the default entry file instead?

MichaelDeBoey avatar Mar 09 '23 00:03 MichaelDeBoey

⚠️ No Changeset found

Latest commit: a02154affd387193f73fb208c86245fc742b1cc7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar Mar 09 '23 01:03 changeset-bot[bot]

ok will do !

Plopix avatar Mar 09 '23 01:03 Plopix

I wonder if entry.server.deno and entry.server.cloudflare deserve the same change ?

Plopix avatar Mar 09 '23 04:03 Plopix

@Plopix I would say they do

You shouldn't change files in templates/remix though, as those will be removed once we merge main into dev (they're already gone on main, see #5455)

MichaelDeBoey avatar Mar 09 '23 11:03 MichaelDeBoey

Hmm... my understanding is that HTTP2 Server Push is being deprecated. Chrome is removing support for it in an upcoming release. Apparently, there are not enough implementations on the server side to make it worthwhile to maintain support.

So not sure this is worth adding to the default templates.

https://developer.chrome.com/blog/removing-push/

kiliman avatar Mar 09 '23 17:03 kiliman

yes that's true, the new stuff to look for is probably: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/103 Already discussed here: https://github.com/remix-run/remix/discussions/5378

Plopix avatar Mar 13 '23 15:03 Plopix

Hi @Plopix!

Are you still interested in getting this one merged?

If so, please rebase onto latest dev & resolve conflicts

MichaelDeBoey avatar May 01 '23 23:05 MichaelDeBoey

Hi everyone!

Depending on @Plopix availability for this, I'm volunteering to help getting it merged if needed. It looks like most of the work is done, and that would be a great addition to Remix.

Please let me know if help is needed here.

real34 avatar May 11 '23 08:05 real34

I am a bit confused actually here. Where should I do changes finally? At some point in this PR I have been told to do not change anything in templates/ as the files will be removed. is that still the case ? entry.client and entry.server seems to be back in main and dev.

Plopix avatar May 12 '23 21:05 Plopix

THIS IS AMAZING!

anaibol avatar Jul 14 '23 22:07 anaibol

I'm going to close this out due to https://github.com/remix-run/remix/pull/3200#issuecomment-1462473053 and because we have an open proposal that will go through our Open Develoment Process in https://github.com/remix-run/remix/discussions/5378.

Also because this can be done today in userland in your entry.server.tsx file.

brophdawg11 avatar Aug 22 '23 20:08 brophdawg11