Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate from sprockets to propshaft #8

Merged
merged 3 commits into from May 16, 2022
Merged

Migrate from sprockets to propshaft #8

merged 3 commits into from May 16, 2022

Conversation

tvararu
Copy link
Contributor

@tvararu tvararu commented May 16, 2022

The problem

Running a fresh app in production mode (RAILS_ENV=production RAILS_SERVE_STATIC_FILES=true) with precompiled assets fails to serve the govuk-crest and the font.

The reason

Sprockets doesn't combine well with cssbundling-rails; it's in a weird halfway spot where it handles fingerprinting but it doesn't touch any of our CSS or assets that are included via embedded url()s. It is capable of moving files from app/assets/builds to public/assets and fingerprinting them, but it is helpless to actually go in and modify the CSS that dart-sass produces, and to replace asset locations with fingerprinted ones.

govuk-frontend SASS API functions such as $govuk-image-url-function don't help like they do with the previous webpacker setup we used, because the fingerprints live inside sprockets and it doesn't talk to dart-sass.

Long discussion of similar issues and potential solutions (I tried the asset preprocessor route myself, but couldn't make it work): rails/cssbundling-rails#22

The solution

rails/propshaft is the exact solution to this problem from the Rails core team. It's a much simpler asset delivery pipeline and with very little configuration accomplishes everything we need:

  • We tell it where the govuk-frontend assets live
  • The ones that are referenced by asset_paths in our Ruby code get copied over and fingerprinted
  • The ones that are referenced by absolute pathed urls like url("/images/govuk-crest-2x.png") in our CSS also get copied over and fingerprinted
  • That's it

Changes proposed in this pull request

Two unrelated commits:

  • Update the gitignore template, it was missing some stuff like tmp etc
  • Remove hotwire/turbo from the default build; was added accidentally previously

Then roughly following the migration guide.

  • Replace sprockets-rails with propshaft in the Gemfile (including the comment which is useful for people new to this library)
  • Add govuk-frontend assets to pipeline: config.assets.paths << Rails.root.join('node_modules/govuk-frontend/govuk/assets')
  • Remove stuff that isn't necessary anymore:
  • Add $govuk-assets-path: "/"; so that URLs look like url("/images/govuk-crest-2x.png") and map correctly; this is the propshaft convention. rails assets:reveal shows all available assets from all sources

Screenshots

Running the following in a fresh project:

RAILS_ENV=production rails assets:precompile
RAILS_ENV=production RAILS_SERVE_STATIC_FILES=true rails server
Before (sad! no fonts! 馃槶 ) After (happy! yes fonts! 馃帀 )
image image

@tvararu tvararu requested a review from peteryates May 16, 2022 11:01
@tvararu
Copy link
Contributor Author

tvararu commented May 16, 2022

Here is a git diff of the changes that the template does to a fresh project (lockfiles omitted); it's both a way to review the "results" of running this new code, as well as a DIY guide to how to do this migration if you don't use this template:

diff --git a/Gemfile b/Gemfile
index 7178277..ffbff17 100644
--- a/Gemfile
+++ b/Gemfile
@@ -6,8 +6,8 @@ ruby "3.1.2"
 # Bundle edge Rails instead: gem "rails", github: "rails/rails", branch: "main"
 gem "rails", "~> 7.0.3"

-# The original asset pipeline for Rails [https://github.com/rails/sprockets-rails]
-gem "sprockets-rails"
+# The newer and simpler asset pipeline for Rails [https://github.com/rails/propshaft]
+gem "propshaft"

 # Use sqlite3 as the database for Active Record
 gem "sqlite3", "~> 1.4"
diff --git a/app/assets/config/manifest.js b/app/assets/config/manifest.js
deleted file mode 100644
index 99fcd3a..0000000
--- a/app/assets/config/manifest.js
+++ /dev/null
@@ -1,2 +0,0 @@
-//= link_tree ../builds/images
-//= link_tree ../builds/
diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss
index fc0ffcd..1370358 100644
--- a/app/assets/stylesheets/application.scss
+++ b/app/assets/stylesheets/application.scss
@@ -1 +1,5 @@
-@use "govuk-frontend/govuk/all";
+$govuk-global-styles: true;
+$govuk-new-link-styles: true;
+$govuk-assets-path: "/";
+
+@import "govuk-frontend/govuk/all";
diff --git a/config/application.rb b/config/application.rb
index 8f44698..7468e99 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -35,5 +35,7 @@ module TestProject
     config.generators.system_tests = nil

     config.exceptions_app = routes
+
+    config.assets.paths << Rails.root.join('node_modules/govuk-frontend/govuk/assets')
   end
 end
diff --git a/config/initializers/assets.rb b/config/initializers/assets.rb
deleted file mode 100644
index 2eeef96..0000000
--- a/config/initializers/assets.rb
+++ /dev/null
@@ -1,12 +0,0 @@
-# Be sure to restart your server when you modify this file.
-
-# Version of your assets, change this if you want to expire all your assets.
-Rails.application.config.assets.version = "1.0"
-
-# Add additional assets to the asset load path.
-# Rails.application.config.assets.paths << Emoji.images_path
-
-# Precompile additional assets.
-# application.js, application.css, and all non-JS/CSS in the app/assets
-# folder are already added.
-# Rails.application.config.assets.precompile += %w( admin.js admin.css )
diff --git a/package.json b/package.json
index 4e40760..3e90886 100644
--- a/package.json
+++ b/package.json
@@ -2,16 +2,12 @@
     "sass": "^1.49.8"
   },
   "scripts": {
     "build:css": "sass --embed-sources --quiet-deps --load-path=node_modules ./app/assets/stylesheets/application.scss ./app/assets/builds/application.css",
-    "build:js": "esbuild app/javascript/*.* --bundle --outdir=app/assets/builds",
-    "preinstall": "mkdir -p app/assets/builds/{fonts,images}",
-    "postinstall": "cp -R node_modules/govuk-frontend/govuk/assets/fonts/. app/assets/builds/fonts && cp -R node_modules/govuk-frontend/govuk/assets/images/. app/assets/builds/images"
+    "build:js": "esbuild app/javascript/*.* --bundle --outdir=app/assets/builds"
   }
 }

Comment on lines -14 to -15
"preinstall": "mkdir -p app/assets/builds/{fonts,images}",
"postinstall": "cp -R node_modules/govuk-frontend/govuk/assets/fonts/. app/assets/builds/fonts && cp -R node_modules/govuk-frontend/govuk/assets/images/. app/assets/builds/images"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So much nicer! 馃帀

Copy link
Member

@peteryates peteryates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definite improvement, looks great

@tvararu tvararu merged commit b32c540 into main May 16, 2022
@tvararu tvararu deleted the propshaft branch May 16, 2022 11:24
tvararu added a commit to DFE-Digital/apply-for-qualified-teacher-status that referenced this pull request May 16, 2022
This changeset was auto-generated by the `rails-template`:
DFE-Digital/rails-template#8
tvararu added a commit to DFE-Digital/apply-for-qualified-teacher-status that referenced this pull request May 16, 2022
This changeset was auto-generated by the `rails-template`:
DFE-Digital/rails-template#8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants