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

Proposal: Rename Vnd.Prefetch header set when preloading links #924

Closed
pfeiffer opened this issue May 24, 2023 · 4 comments
Closed

Proposal: Rename Vnd.Prefetch header set when preloading links #924

pfeiffer opened this issue May 24, 2023 · 4 comments

Comments

@pfeiffer
Copy link
Contributor

pfeiffer commented May 24, 2023

Introduced in #552 by @hey-leon is the ability to preload links tagged with [data-turbo-preload]. This works great for prepopulating the cache. The links are fetched with the header Vnd.Prefetch set to true, which the server can use to determine that the request was made as a preloaded request. To be honest it's not clear where the Vnd.Prefetch name for the header originated - it was introduced in the PR and I see no mention of it elsewhere than in the Turbo PR.

One gotcha with this is that CloudFlare seems to strips all request headers containing dots. It seems like this also happens for other vendors as the header name is invalid(?). For instance Oracle, nginx etc. An easy fix would be to rename the header to something else that doesn't contain a dot.

At the same time it could be worth considering if the header should be renamed to something that conforms with what other vendors use. Safari and Mozilla uses a (X-)Purpose: prefetch for prefetching <link prefetch ..>

Chrome, Safari and Facebook in-app browser use a X-Purpose: preview header to indicate prefetching a preview of a page before the user navigates to it. I think this would be very similar to how the Turbo preloading happens, as Turbo per default triggers a re-fetch of a preloaded page when tapping the preloaded link.

TLDR; It seems like vnd.prefetch header is invalid and stripped by many vendors. Setting X-Purpose: preview would probably be a better option and conform with the behavior of other vendors for fetching a light preview of a page before fetching it fully.

Happy to provide a PR that changes this!

@rik
Copy link
Contributor

rik commented May 24, 2023

The current standard for this is Sec-Purpose: prefetch. Browsers might not yet be up-to-date with the spec but if possible, I think using that header would be more forward looking.

@hey-leon
Copy link
Contributor

hey-leon commented May 25, 2023

@pfeiffer good call out.

I think I was mistaken when adding this header. Some samples of code I found before implementing this feature included it, and I believe I included it under a false assumption it was used by turbo. If you search the repo on github for the header it does not come up (other than the case I added). I think we are probably good to simply remove the header or opt for a standardised version.

I am not a member on this repo though so I think we are waiting on someone from the team. If you haven't already it may be worth posting on the discord - https://discord.com/channels/988103760050012160/1044659721229054033

@seanpdoyle
Copy link
Contributor

With #925 being merged, is it safe to close this issue @pfeiffer @afcapel ?

@pfeiffer
Copy link
Contributor Author

With #925 being merged, is it safe to close this issue @pfeiffer @afcapel ?

Yeah, we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants