Guard [data-turbo-preload] with conditionals#1033
Conversation
7c9cb74 to
279bbfc
Compare
Related to [@hotwired/turbo#1033][] Document the cases when `[data-turbo-preload]` has no effect. [@hotwired/turbo#1033]: hotwired/turbo#1033
|
I've opened hotwired/turbo-site#150 to make these edge cases more obvious. |
9462211 to
49c1b86
Compare
49c1b86 to
184fc5e
Compare
184fc5e to
147e70c
Compare
Prior to this change, _any_ `<a>` element with `[data-turbo-preload]` would be preloaded. With that behavior comes some risk: * if an element is nested within a `[data-turbo="false"]` element, the preloading would occur unnecessarily * if an element has `[data-turbo-stream]`, the preloaded request won't be the same as the ensuing request due to differences in the `Accept:` header * if an element has `[data-turbo-method]`, the preloaded request won't be the same as the ensuing request due to differences in the method * if an element is within a `<turbo-frame>` or driving a frame via `[data-turbo-frame]`, the preloaded request won't be the same as the ensuing request due to differences in the `Turbo-Frame:` header This commit extends the `Preloader` delegate interface to include a `shouldPreloadLink(link: HTMLAnchorElement)` predicate method to give delegates an opportunity to opt-out of preloading. The `Session` implementation of the delegate class adds rejects `<a>` elements that match any of those cases. Co-authored-by: Julian Rubisch <julian@julianrubisch.at>
147e70c to
05ed1b7
Compare
|
@afcapel @jorgemanrubia this is ready for review, if either of you are available. |
|
Thanks @seanpdoyle, nice one! |
|
@seanpdoyle I'm trying to achieve some preloading behaviour that appears to be impossible with the changes made here, specifically: const frame = frameTarget == "_top" ?
null :
document.getElementById(frameTarget) || findClosestRecursively(element, "turbo-frame:not([disabled])");My understanding is:
Then if it's present and a kind of In my example, I have tabbed content within a page containing other content. I wish for users to be able to click the tabs and for the content to be shown instantly since it has been prefetched. See my diagram for more information: If I set my In Turbo
However in Turbo My suggestion might be nieve as I'm not completely clear on the why this condition was added. I was thinking though that:
The code change in const frame = frameTarget == "_top" ?
null :
- document.getElementById(frameTarget) || findClosestRecursively(element, "turbo-frame:not([disabled])");
+ document.getElementById(frameTarget);
+
+ if (frame && frame !== findClosestRecursively(element, "turbo-frame:not([disabled])")) {
+ return false;
+ }I'd appreciate if you could let me know what you think and I'd he happy to submit a PR if you think this change makes sense or if there is a different way of solving my problem. |
Related to [@hotwired/turbo#1033][] Document the cases when `[data-turbo-preload]` has no effect. [@hotwired/turbo#1033]: hotwired/turbo#1033 Co-authored-by: Matheus Richard <matheusrichardt@gmail.com>
Prior to this change, _any_ `<a>` element with `[data-turbo-preload]` would be preloaded. With that behavior comes some risk: * if an element is nested within a `[data-turbo="false"]` element, the preloading would occur unnecessarily * if an element has `[data-turbo-stream]`, the preloaded request won't be the same as the ensuing request due to differences in the `Accept:` header * if an element has `[data-turbo-method]`, the preloaded request won't be the same as the ensuing request due to differences in the method * if an element is within a `<turbo-frame>` or driving a frame via `[data-turbo-frame]`, the preloaded request won't be the same as the ensuing request due to differences in the `Turbo-Frame:` header This commit extends the `Preloader` delegate interface to include a `shouldPreloadLink(link: HTMLAnchorElement)` predicate method to give delegates an opportunity to opt-out of preloading. The `Session` implementation of the delegate class adds rejects `<a>` elements that match any of those cases. Co-authored-by: Julian Rubisch <julian@julianrubisch.at>

Closes #857
Prior to this change, any
<a>element with[data-turbo-preload]would be preloaded. With that behavior comes some risk:[data-turbo="false"]element, the preloading would occur unnecessarily[data-turbo-stream], the preloaded request won't be the same as the ensuing request due to differences in theAccept:header[data-turbo-method], the preloaded request won't be the same as the ensuing request due to differences in the method<turbo-frame>or driving a frame via[data-turbo-frame], the preloaded request won't be the same as the ensuing request due to differences in theTurbo-Frame:headerThis commit extends the
Preloaderdelegate interface to include ashouldPreloadLink(link: HTMLAnchorElement)predicate method to give delegates an opportunity to opt-out of preloading. TheSessionimplementation of the delegate class adds rejects<a>elements that match any of those cases.