bigskysoftware / htmx-extensions

164 stars 50 forks source link

preload: added a "hx-preload" header to hx-get and href requests #61

Open bogdano opened 4 months ago

bogdano commented 4 months ago

Also added hx-request header to href preload requests, in case implementations rely on hx-boost + hx-target to update a #main div, for instance.

If UI elements hook into htmx:configRequest, then a preload request may trigger some client-side action before the user actually commits to executing the request. A check for an hx-preload header can be used to avoid this.

Also, if an implementation uses the pattern of hx-boosted <a href=""> tags and server-side logic to return only partials when a request contains the hx-request header, then the preload would unnecessarily load an entire page when a snippet would suffice. So, I've added an hx-request and hx-preload header to the XMLHTTPRequest as well.

Please let me know what you think about this change.

netlify[bot] commented 4 months ago

Deploy Preview for htmx-extensions canceled.

Name Link
Latest commit fb620072e59fa0cc80e9572794d86396c3fce9ad
Latest deploy log https://app.netlify.com/sites/htmx-extensions/deploys/66a121f96dc9740008c1553e
bogdano commented 4 months ago

I apologize if I should have just opened an issue instead-- I didn't see any contribution guidelines until after I submitted the PR and checked the base htmx repo...

Telroshan commented 4 months ago

Hey,

ALameLlama commented 2 weeks ago

I was just about to open a PR for this, I believe this is be HX-Preloaded to remain consistent with HX-Boosted

If @bogdano doesn't update their PR, can I submit a new one for this? @Telroshan

bogdano commented 2 weeks ago

Yes, I am really sorry about leaving this alone until now. Would you mind giving me another week to do this? I'd love to contribute; I've just been preoccupied with other things in my life recently.

Also, I agree that HX-Preloaded makes more sense, consistency-wise.

ALameLlama commented 2 weeks ago

No stress, I only created a patch which included the headers and haven't started on the tests.

These are some of the ideas for tests I had.

marisst commented 2 weeks ago

@Telroshan I believe that this PR should be closed without merging.

@bogdano HTMX request headers should not be added by the preload extension. This logic should stay with the core HTMX library. The problem is that the preload extension currently incorrectly decides when to preload content using XML or HTMX request, especially in the case of hx-boost. I have fixed the issue in #106 preload.js making sure that all the other HTMX request headers are also passed along correctly in the request e.g. HX-Current-URL. I have also written tests, to make sure the request headers are the same when preloading and loading the element.

@ALameLlama please have a look at #106 and let me know if this is what you were thinking? I have added the tests for what you have mentioned in your comment. Sorry, I already started working on the changes last week and did not see your comment from Monday.

ALameLlama commented 1 week ago

@marisst, I'll have a look at this on Monday since I'll be able to test it with the codebase. The main thing I need is a way from the BE to be able to tell if it was a preload request, boosted request, or a normal request.

if it's a preload request I change the Cache-Control in the response header to 30 seconds so we can cache it client side with the preload extension otherwise we normally don't have any client side.

This is why I was originally looking at doing something similar to the HX-Boosted request header since we also have conditional logic if the request has this header.

ALameLlama commented 1 week ago

@marisst Tests look great IMO and I tried it in my code base and it mostly works. I noticed that preloads now fire off hx indicators which I don't believe it should be doing and I still think sending a separate header just for Preloaded would be handy so the backend.

@Telroshan I still believe this PR is relevant.

marisst commented 1 week ago

@ALameLlama Thank you! I have addressed all your comments in https://github.com/bigskysoftware/htmx-extensions/pull/106.