hotwired / turbo

The speed of a single-page web application without having to write any JavaScript
https://turbo.hotwired.dev
MIT License
6.73k stars 429 forks source link

Last-modified and if-modified-since #813

Open aleho opened 1 year ago

aleho commented 1 year ago

Is there a way for Turbo to handle last-modified and if-modified-since?

My application can send a last-modified and caching headers. On a full page request the browser will send an if-modified-since header and the application will not have to build / render / do whatever takes CPU.

When Turbo requests a page I haven't seen an if-modified-since header.

Did I miss something?

seanpdoyle commented 1 year ago

Turbo's Fetch Requests aren't made with any deliberate inclusion of those headers:

https://github.com/hotwired/turbo/blob/main/src/http/fetch_request.ts#L141-L157

Have you tried declaring a turbo:before-fetch-request event listener that writes to the Fetch Request's headers?

aleho commented 1 year ago

No. I'm vaguely aware of these events, but my impression was that I'd have to keep a separate list for every cached page and its header, if any.

Is there a way to do this elegantly using Turbo's storage?

seanpdoyle commented 1 year ago

Turbo's classes for Visit (what drives a full-page visit) and FrameController (what drives a Turbo Frame visit) both have opportunities to modify their request's headers:

https://github.com/hotwired/turbo/blob/main/src/core/drive/visit.ts#L338-L342

https://github.com/hotwired/turbo/blob/main/src/core/frames/frame_controller.ts#L247-L253

Just for my own understanding, are you asking if Turbo could set their underlying requests' Last-Modified and If-Modified-Since headers?

If Turbo were to do that, where would it read values from? Would they be part of a prior Fetch request's response, or would Turbo need to maintain its own mapping from URL to Last-Modified value?

aleho commented 1 year ago

Turbo's classes for Visit (what drives a full-page visit) and FrameController (what drives a Turbo Frame visit) both have opportunities to modify their request's headers:

https://github.com/hotwired/turbo/blob/main/src/core/drive/visit.ts#L338-L342

https://github.com/hotwired/turbo/blob/main/src/core/frames/frame_controller.ts#L247-L253

Thanks! I'll have a more in-depth look at the headers. The challenge will be to tell it not to use the response at all, because of caching.

Just for my own understanding, are you asking if Turbo could set their underlying requests' Last-Modified and If-Modified-Since headers?

If Turbo were to do that, where would it read values from? Would they be part of a prior Fetch request's response, or would Turbo need to maintain its own mapping from URL to Last-Modified value?

Yes, that's my idea.

Let's assume a fetch response contains a last-modified or etag header (off the top of my head I don't see a reason why it couldn't).

Both, etag and last-modified could then be stored along the history entry.

Upon the next fetch request, or history navigation, Turbo could find these headers and set them (if-modified-since, if-none-match). It would then still be up to the backend to decide what to do, but in my case a 304 response is sent without having to render the page or set any content.

Turbo could then detect the 304 and won't have to update the page content.

Does this sound reasonable?

nicolas-grekas commented 2 weeks ago

On first-visit rendering, it'd be great to read those headers from some data-attribute, so that we could prerender a frame and let Turbo refresh it using HTTP-cache semantics.

aleho commented 2 weeks ago

I just tested a little bit and as it currently seems to work, Turbo uses fetch with default cache setup. So if the backend sends a Last-Modified, fetch will send an If-Modified-Since header (tested in Firefox). I tried with _top and frame targets in links as well as browser history navigation.

It's been almost two years since I tested so I'm confused right now. Did this always work? Were my initial assumptions wrong? Did I test in different circumstances?

Can someone confirm this is actually a non-issue?