Comments (13)
@steerio I've opened #1198 to try and resolve this issue. I've added tests that aim to reproduce the issue as you've described it.
Unfortunately, they pass without any implementation changes. Could you provide some feedback on that PR to make those tests fail consistently?
from turbo.
Thank you for opening this issue. I've tried to reproduce it on main
with the following change:
index f192590..c31bfc0 100644
--- a/src/tests/functional/frame_tests.js
+++ b/src/tests/functional/frame_tests.js
@@ -2,6 +2,7 @@ import { test, expect } from "@playwright/test"
import { assert, Assertion } from "chai"
import {
attributeForSelector,
+ getSearchParam,
hasSelector,
listenForEventOnTarget,
nextAttributeMutationNamed,
@@ -82,6 +83,21 @@ test("following a link sets the frame element's [src]", async ({ page }) => {
assert.equal(src.searchParams.get("key"), "value", "[src] attribute encodes query parameters")
})
+test("setting the element's [src] navigates the frame", async ({ page }) => {
+ const frame = page.locator("#frame")
+
+ await expect(frame.locator("h2")).toHaveText("Frames: #frame")
+ await frame.evaluate((frame) => frame.src = "/src/tests/fixtures/frames/frame.html?key=value")
+
+ const { url, fetchOptions: { headers } } = await nextEventOnTarget(page, "frame", "turbo:before-fetch-request")
+
+ expect(headers["Turbo-Frame"]).toEqual("frame")
+ expect(pathname(url)).toEqual("/src/tests/fixtures/frames/frame.html")
+ expect(getSearchParam(url, "key"), "fetch request encodes query parameters").toEqual("value")
+
+ await expect(frame.locator("h2")).toHaveText("Frame: Loaded")
+})
+
test("following a link doesn't set the frame element's [src] if the link has [data-turbo-stream]", async ({ page }) => {
await page.goto("/src/tests/fixtures/form.html")
Unfortunately, I was not able to fail the test. Does it reflect the circumstances in which you've encountered this unexpected behavior?
In case you want to try it locally, here's the block:
test("setting the element's [src] navigates the frame", async ({ page }) => {
const frame = page.locator("#frame")
await expect(frame.locator("h2")).toHaveText("Frames: #frame")
await frame.evaluate((frame) => frame.src = "/src/tests/fixtures/frames/frame.html?key=value")
const { url, fetchOptions: { headers } } = await nextEventOnTarget(page, "frame", "turbo:before-fetch-request")
expect(headers["Turbo-Frame"]).toEqual("frame")
expect(pathname(url)).toEqual("/src/tests/fixtures/frames/frame.html")
expect(getSearchParam(url, "key"), "fetch request encodes query parameters").toEqual("value")
await expect(frame.locator("h2")).toHaveText("Frame: Loaded")
})
from turbo.
Thanks, Sean, I'll check that.
In the meantime I created this minimal Rails app to showcase the issue.
from turbo.
@steerio thank you for creating that repository!
As a piece of advice, git diffs can be really useful in reproduction repositories. In its current format, that repository has a single commit that includes both rails new
-generated content alongside the specific changes you've made to demonstrate the issue. It's almost impossible to separate the two types of code at a glance without reading every Ruby, JavaScript, or HTML file and mentally comparing them to what I expect a new Rails application to have.
Could you revise the repository so that the "Intial commit" only contains the rails new
code, then a second commit only contains the changes necessary to reproduce this behavior?
from turbo.
All right, I've done this and made a force push to the same repo, and here is the commit with the changes.
To see it in action you only need to bundle install
and then rails s
. Everything is on the front page.
It's worthy to note that if you downgrade turbo-rails with this change in the Gemfile, it all starts working.
-gem "turbo-rails"
+gem "turbo-rails", "<2"
from turbo.
I noticed something interesting with the demo code:
- The page is already loaded on mouseover. Looks like this is InstaClick, which indeed has been introduced by v8.
- The first click makes no request, hinting that the preload is being reused.
- If you make two clicks (while managing to stay over the link), the second one will work properly.
So the problem is that the InstaClick preload is used even when the final request is not exactly the same as the preload one. In our case, when the request is preloaded, Turbo expects a visit to happen (and can't have any idea about our onclick intentions), so that request is made without the header we're expecting to be present.
The second click (provided that you didn't leave the link with the mouse before it) will make a proper request.
I think the situation can be rectified by adding the headers to the cache key. For the time being, I'll switch it off with a meta tag - even with that fix, we'd have two requests done instead of one.
from turbo.
To add a bit of an explanation: our use case is that some pages can be loaded in a modal and standalone as well. To tell these situations apart (and to give an ID to the Turbo Frame holding the content) we're using the Turbo-Frame
header.
We also try to cut down on payload sizes for larger, Turbo-heavy pages, so in case of Turbo Frame requests, we try to only render what was requested.
We defined some handy helpers to help with this kind of thing:
requested_frame
: string, the value of the header basically.frame_request?
: boolean, whether the header was present.frame_wanted?(id)
: boolean, true if the header equals to the argument or if the header was missing
It all worked well until turbo-rails 2 bumped the version of Turbo to v8.
from turbo.
I think you're correct that pre-fetching is at the root of this behavior.
In the meantime, are you able to render <meta name="turbo-prefetch" content="false">
into your <head>
element?
from turbo.
Yes, that's what I ended up doing, and it works.
from turbo.
Looping in @afcapel @davidalejandroaguilar to help troubleshoot this behavior.
from turbo.
@seanpdoyle I don't readily have a system that would be able to run these tests (Arch has some issues with libpcre.so.3 and they didn't work on a Mac for some reason either). I'll set up a Debian in Docker, have them all pass, and see if I can write one that fails on this issue.
Just by glancing over it I'd say that it probably works with frame requests that it triggers itself, i.e. even at mouseover time it knows that when clicked, it would load in a given frame. In my case there was a Stimulus controller (which could be any event listener) on the click event with custom code that would do that.
This custom code simply updates the src
attribute on a Turbo frame, and this should not blindly reuse the mouseover cache. Also, there could be a way to disable the prefetch feature on a per-link basis, because if this bug is fixed, there's still a useless extra request creating unnecessary load.
from turbo.
there could be a way to disable the prefetch feature on a per-link basis
Have you tried adding the [data-turbo-prefetch="false"]
attribute to the link?
from turbo.
I haven't - I disabled the whole thing (which is fairly new to me, having only recently updated) via the meta tag. This looks like a nice alternative, thanks for pointing it out.
from turbo.
Related Issues (20)
- Script tags are reexecuted when navigating back from a page with an error status code using browser's back button HOT 1
- Eager-loaded frame fetches but does not render its src HOT 1
- Server-side redirect from turbo stream to file download fails in Turbo 8
- Firefox issue with `scroll-behavior: smooth` and Turbo enabled (v8.0.4) HOT 3
- The documentation does not mention that Stream Action tags can be rendered inside any HTML HOT 5
- using data-turbo-action "replace" or "advance" cause turbo:before-cache event to be called on live page HOT 2
- Occasional disconnect, no documented way to monitor or reconnect HOT 5
- Incorrect page content when a Turbo "replace" stream action is followed by a refresh
- turbo-confirm doesn't work on links without a turbo-method HOT 1
- Window.fetch has been overwritten by Stimulus HOT 5
- Adding stimulus breaks a test...why?
- Indefinite Progress Bar on Turbo.visit with turbo-stream content HOT 5
- Preserve scroll position when morphing turbo frames
- turbo:morph-element fires twice
- Form not displaying errors on page HOT 2
- Morph with infinite scroll
- turbo-frame with text only not rendering on restoration visits
- How to prevent element from being removed during morph? HOT 3
- data-turbo="false" misbehaviour HOT 1
- Drive: Navigating back from a 3xx redirect to an external page makes turbo show 'loading' forever
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from turbo.