Migrate Turbolinks to Turbo #2062
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MrSerth
added
dependencies
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2062 +/- ##
==========================================
- Coverage 94.76% 94.74% -0.03%
==========================================
Files 133 133
Lines 3400 3404 +4
==========================================
+ Hits 3222 3225 +3
- Misses 178 179 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
MrSerth
force-pushed
the
ss/turbo-migration
branch
from
2d6b936 to
9f854ca
Compare
This is the first part of using Turbo for this code base. Some solutions, like the temporary events triggered, should be temporary and removed once we fully migrated from Sprockets to another JavaScript distribution (such as Shakapacker, importmaps, etc.)
These links work correctly with Turbo, so that we don't need to opt out here.
Turbo imposes a new requirement for form submissions: Whenever a form is submitted, a redirect is expected. When no redirect is performed (i.e., when the data submitted didn't pass validation and the same form is rendered again), a 4xx or 5xx error code is required. Therefore, this commit changes those occurrences to return a 422 error code. Without this change, an error would be logged by Turbo in the JavaScript console and the response (including the flash message) wouldn't be shown. See https://turbo.hotwired.dev/handbook/drive#redirecting-after-a-form-submission
Turbo requires us to use the HTTP status code 303 for successful form submissions after stateful server changes. See https://turbo.hotwired.dev/handbook/drive#redirecting-after-a-form-submission
For these actions, we use use a non-default `method` (other than `GET`). To indicate that within the DOM (i.e., for Screenreaders) but also enhance compatibility with Turbo, we switch to proper `button_to` forms here. As part of this migration, some buttons should still be shown inline (hence, we add the `form_class: 'd-inline-block'`) or with the same height as regular links (hence, we add `class: 'h-100'`). See https://github.com/heartcombo/devise/wiki/How-To:-Upgrade-to-Devise-4.9.0-%5BHotwire-Turbo-integration%5D#data-method-vs-data-turbo-method See https://gorails.com/episodes/link_to-vs-button_to-in-rails
Similar to all other controllers, Devise also needs to respect the updated HTTP status codes for Turbo. Luckily, we use the newest Devise version already, which comes with built-in support for Turbo already. See https://github.com/heartcombo/devise/wiki/How-To:-Upgrade-to-Devise-4.9.0-%5BHotwire-Turbo-integration%5D
When the form is submitted but no redirect occurs (e.g., after a validation error), the page content is simply replaced. However, for this occasion Turbo misses to emit a new `turbo:load` event, which makes it more difficult to use custom JavaScript waiting on page initializations. Potentially, we could use `turbo:render` here to fix these occasions.
With Turbo enabled on forms with custom JavaScript / CSS packs and a successful change (no validation error), the final page would be rendered twice: The first rendering would occur after sending the form data using Turbo. This response is fetched and potentially displayed (first visit). The resulting DOM contains a change in the packs (with `'data-turbo-track': 'reload'`), causing a page reload (without Turbo). This is the second visit. Through this "double-visit", any flash message intended to be shown after the form submission is simply "lost". Therefore, we disable Turbo in these cases. This holds also true for signout actions, that can (potentially) originate from any page, including those with custom packs, and redirect to the root page (without any additional packs).
With this new method, Tooltips are correctly initialized and potential loading errors/race conditions are prevented.
With this change, each editor instance is destroyed upon changing the page. This prevents memory leaks and thus optimizes the performance. The editor will be restored upon visiting the sites again.
Similar to the ACE editor, we unload Select2 to prevent memory leaks and duplicated selectors when navigating to a cached site.
In general, we can run system specs with four browser combinations: 1. Chrome 2. Chrome Headless 3. Firefox 4. Firefox Headless Unfortunately, Selenium has an ongoing issue with Chrome Headless and Alerts, making the test execution slow or causing random failures. Non-headless Chrome works fine. To overcome these issues, we switch to Firefox for the time being in GitHub Actions. Non-headless browsers are not supported in GitHub Actions.
MrSerth
force-pushed
the
ss/turbo-migration
branch
from
9f854ca to
4db7b14
Compare
arkirchner
approved these changes
Jul 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first part of using Turbo for this code base. Some solutions, like the temporary events triggered, should be temporary and removed once we fully migrated from Sprockets to another JavaScript distribution (such as Shakapacker, importmaps, etc.). More details are available in the dedicated commit messages. Related to CodeOcean #2952. While I have tested the changes extensively, handling of the JavaScript events is not that easy and might require additional refinements later (based on Sentry events or other observations).