Remove EOL turbolinks#86
Merged
Merged
Conversation
turbolinks has been end-of-life since 2019. This app is a two-page, Sprockets-based form tool with no need for SPA-style navigation, Turbo Frames, or Streams, so it's dropped outright rather than replaced with a JS framework. (turbo-rails 2.x ships ESM-only JS that can't be //= require'd into a Sprockets classic-script bundle, so adopting it here would mean an importmap/bundler migration -- deliberately out of scope.) - Gemfile: remove gem "turbolinks" - application.js: drop //= require turbolinks (nothing replaces it) - header.js / form.js / utils.js: the handlers previously bound inside a turbolinks:load callback; rebind interactive handlers via jQuery event delegation on document (bind once, survive full page loads) and run the on-display init (sticky header, read-more) on DOM ready. No dependency on any navigation-library load event. Verified locally: boots, assets:precompile clean (no turbolinks/turbo in the bundle), unit tests 16/52/0, rubocop + reek clean, and the system test (upload -> results -> PDF) passes in a real headless Chrome.
JuanVqz
force-pushed
the
chore/replace-turbolinks-with-turbo
branch
from
34c1443 to
10f065a
Compare
JuanVqz
changed the title
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
Removes the EOL
turbolinksgem. It is dropped outright, not replaced with a JS framework.Why
turbolinkshas been end-of-life since 2019. This app is a two-page, Sprockets-based form tool with no need for SPA-style navigation, Turbo Frames, or Turbo Streams, so removing it (full page loads) is the right fit.Adopting
turbo-railswas attempted first but reverted: turbo-rails 2.x ships ESM-only JavaScript (export {...}), which cannot be//= required into a Sprockets classic-script bundle — theexportis aSyntaxErrorthat breaks the wholeapplication.js. Making Turbo work here would require migrating the asset pipeline to importmap or a JS bundler, which is deliberately out of scope for this cleanup.Changes
Gemfile: removegem "turbolinks"application.js: drop//= require turbolinks(nothing replaces it)header.js/form.js/utils.js: these handlers were bound inside aturbolinks:loadcallback. Rebind the interactive handlers (file-input change, navbar/menu clicks, smooth-scroll) via jQuery event delegation ondocument(bind once, survive full page loads), and run the on-display init (sticky header, read-more visibility) on DOM ready. No dependency on any navigation-library load event.Gemfile.lock+ dual-bootGemfile.next.lockupdatedVerification
assets:precompileclean (no turbolinks/turbo in the bundle)rubocopclean (exact CI config),reekcleanupload → results → PDF) passes in a real headless Chrome (run locally against Chrome/chromedriver 150) — this is the check that matters for a navigation changeFollow-up
A separate PR will convert the app's JavaScript from jQuery to vanilla JS (kept out of this focused change).