Turbo Preloads should not be prevented when using the closest Turbo Frame

[adambutler]

I'm reporting what I think is an issue introduced in #1033. The aim of this change as I understand it is to prevent unnecessary preloading of Turbo links where turbo-preload is present. However, I think it goes too far and prevents preloads for Turbo Frame requests that I think should be allowed. I have a practical use-case where this behaviour is desired documented further down.

The preloading behaviour appears to be prevented specifically by this line:

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 if this is by design or agree if it's a bug and if so I'd he happy to submit a PR. Alternatively, if there is a different way of solving my problem I'd like to understand this better.

[sascha-egerer]

We've got the same issue. We have Frame that contains links which should be preloaded. This does not work anymore since #1033 but has worked without problems before.