Guard [data-turbo-preload] with conditionals

[seanpdoyle]

Closes #857

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.

[seanpdoyle]

I've opened hotwired/turbo-site#150 to make these edge cases more obvious.

[seanpdoyle]

@afcapel @jorgemanrubia this is ready for review, if either of you are available.

[afcapel]

Thanks @seanpdoyle, nice one!

[adambutler]

@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:

1. If the frameTarget is "_top" then frame is set to an explicit null
2. Otherwise; Attempt to find the element by the data-turbo-frame attribute.
3. Otherwise; Get the closest <turbo-frame>

Then if it's present and a kind of FrameElement a prefetch will be prevented.

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 data-turbo-frame="_top" this causes it to work as expected but does mean that I'm loading and replacing the whole document rather than just the frame I'd like.

In Turbo v7.3.0:

• The fetch for the prefetch pages performs as expected
• However, upon clicking the link the cache is not used.
• Next time the same link is used within in the same page visit the cache is used.

However in Turbo v8.0.0-beta.2 I believe the issue of the prefrech populating the cache to be used upon click has been solved by some other PR all that remains is fixing an issue that this PR has seemed to introduce where the prefetch is not invoked due to the guard introduced.

My suggestion might be nieve as I'm not completely clear on the why this condition was added. I was thinking though that:

1. If the frameTarget is "_top:" then it continues to work
2. If the frameTarget is not present it allows it to perform the prefetch (a change in behaviour)
3. If the frameTarget is present
   a. If it matches the closest turbo frame it allows it to perform the prefetch (a change in behaviour)
   b. Otherwise the prefetch is prevented

The code change in src/core/session.js required would be as such:

    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.