Comments (4)
seanpdoyle
commented on August 10, 2024
Thank you for opening this issue.
Are your <a> elements rendered with [data-turbo-stream]? If they are, they should be ignored by the Link Prefetcher.
If you're able, could you modify the script shared below to reproduce your behavior?
bug.rb
require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
git_source(:github) { |repo| "https://github.com/#{repo}.git" }
gem "rails"
gem "propshaft"
gem "sqlite3"
gem "turbo-rails"
gem "capybara"
gem "cuprite", "~> 0.9", require: "capybara/cuprite"
end
require "active_record/railtie"
# require "active_storage/engine"
require "action_controller/railtie"
require "action_view/railtie"
# require "action_mailer/railtie"
# require "active_job/railtie"
# require "action_cable/engine"
# require "action_mailbox/engine"
# require "action_text/engine"
require "rails/test_unit/railtie"
class App < Rails::Application
config.load_defaults Rails::VERSION::STRING.to_f
config.root = __dir__
config.hosts << "example.org"
config.eager_load = false
config.session_store :cookie_store, key: "cookie_store_key"
config.secret_key_base = "secret_key_base"
config.consider_all_requests_local = true
config.turbo.draw_routes = false
Rails.logger = config.logger = Logger.new($stdout)
routes.append do
root to: "application#index"
end
end
class ApplicationController < ActionController::Base
include Rails.application.routes.url_helpers
class_attribute :template, default: DATA.read
def index
render inline: template, formats: :html
end
end
class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
driven_by :cuprite, using: :chrome, screen_size: [1400, 1400], options: { js_errors: true }
end
Capybara.configure do |config|
config.server = :webrick
config.default_normalize_ws = true
end
ENV["DATABASE_URL"] = "sqlite3::memory:"
ENV["RAILS_ENV"] ||= "test"
Rails.application.initialize!
require "rails/test_help"
class TurboSystemTest < ApplicationSystemTestCase
test "reproduces bug" do
visit root_path
assert_text "Loaded with Turbo"
end
end
__END__
<!DOCTYPE html>
<html>
<head>
<%= csrf_meta_tags %>
<script type="importmap">
{
"imports": {
"@hotwired/turbo-rails": "<%= asset_path("turbo.js") %>"
}
}
</script>
<script type="module">
import "@hotwired/turbo-rails"
addEventListener("turbo:load", () => document.body.innerHTML = "Loaded with Turbo")
</script>
</head>
<body>Loaded without Turbo</body>
</html>from turbo-rails.
BenCh94
commented on August 10, 2024
@seanpdoyle adding [data-turbo-stream] to the fixed this issue, thanks for the quick reply
from turbo-rails.
radanskoric
commented on August 10, 2024
I have found this issue because I ran into the same issue. An endpoint may respond with a turbo stream and the turbo stream was being executed on prefetching.
Adding data-turbo-stream indeed works but I am left wondering what is preventing Turbo from ignoring the prefetch result if it comes back as a turbo stream response to avoid having to remember to specify the attribute?
I am thinking that the following 2 statements are true based on the documentation and feature intent:
- Prefetching should never have any effect on the page until the click happens.
- Stream actions always have an effect on the page since that is their sole purpose.
Putting this to together it seems clear that prefetch should always ignore stream action Content-Type response.
Are there any reasons for not implementing this change to remove a non obvious prefetch pitfall?
from turbo-rails.
radanskoric
commented on August 10, 2024
I had a quick debug session to see why this is happening and the short answer is:
StreamObserver#startlistens forbefore-fetch-responseand renders and stream messages into the body to get them to execute. It's doesn't discriminate and does this for all fetch requests. As far as I could tell, it's the only Turbo internal code listening to this event.LinkPrefetchObserver#tryToPrefetchRequestconstructs a regular prefetch request but saves the response without acting on it. However, that happens after the callbacks fire, i.e. too late.
There's no way for the StreamObserver to differentiate fetches that are regular vs those originating from a prefetch.
The easiest way to fix it would be add a property for FetchRequest indicating if it is a prefetch request and then modify behaviour based on it. However that would couple StreamObserver and probably some other classes to the logic of prefetching, making it more expensive to maintain long term.
It would be better if handling of a stream response was decoupled from fetching, same as it is for regular HTML responses.
HTML response gets fetched and is later used by the other code. But a stream response fetching is intercepted and then the response is acted on during the fetching process.
This is fundamentally at opposition to the concept of prefetching. More importantly, it will also be a problem for any future feature that needs request fetching and response handling separated in any way.
I am not yet sure on how the refactoring should look, that would need to be discovered in code. I am interested to try that but before putting effort into it, I'd like to know if this work would be even welcomed by the maintainers?
from turbo-rails.
Related Issues (20)
- InstaClick prefetch not working with fragment cache
- Inverted parameters in stream responses HOT 2
- unexpected routes added as part of the turbo rails gem HOT 3
- When streaming from a worker, path helpers append a domain `https://example.com` HOT 5
- Turbo Documentation need a "Broadcast" chatper
- Refresh broadcasts generated without changes
- Incompatible with Solid_Queue HOT 1
- turbo_frame_request_id safe operator in not handled correctly HOT 2
- Shouldn't broadcast refresh if streamables == [nil] HOT 1
- @npezza93 ActiveRecord::RecordNotFound in MembersController#edit_description
- FORM with an action URL containing a query param is not submitted. HOT 1
- Mixed Content Error with Turbo Drive
- OT: Attackers might be trying to steal your information from discuss.hotwire.dev HOT 2
- Turbo refresh can hijack user navigation HOT 3
- import "@hotwired/turbo-rails" errors HOT 1
- Using data-turbo-confirm on plain (non-form) buttons HOT 3
- Adding stimulus breaks a test HOT 1
- how do you install turboframes into a rails 5 project that migrated to rails 7 using only sprockets (skipping all the webpack & 6 bs)? HOT 2
- What are the controls effected before injecting `text/vnd.turbo-stream.html`?
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-rails.