From fb1ab3460a676ce7def0819c2e92289ef2dcbe3b Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sat, 9 Oct 2021 17:03:05 +0200 Subject: [PATCH] Make Sprockets more optional, offer Propshaft as alternative (#43261) * Make Sprockets more optional, offer Propshaft as alternative * Whups, local reference * No longer used * Spacing * Need explicit sprockets-rails inclusion now * Manually require the sprockets railtie * Don't need these changes right now * Kick off another build * Fix tests * DRY up test * Require railtie when using sprockets * Introduce option to skip asset pipeline * No longer relevant * Always have to return * Gone * Add helper for skip_sprockets? * Fix guard statement * Use latest gems * Include propshaft * fix tests for #43261 (#43277) * help fix tests for #43261 skip_sprockets? should not be called on options :skip_sprockets is no longer a value in the option hash, so skip_sprockets? should not be called on it move --asset-pipeline to shared generator skip_sprockets? is defined on app_base, and used in the plugin generator to determine whether to add the engine's assets to the dummy sprockets manifest, so I believe it makes sense to include in both generators Because of this change, I also changed the shared test back to testing against non-sprockets add skip_sprockets to Gemfile template vars Mocking skip_sprockets? in app_base generator fix more generator tests * use skip_sprockets? everywhere * Use latest propshaft * Update `AssetUrlHelper` docs to list both asset pipeline gems (#43328) * Update to latest Propshaft * Bump Propshaft again * Ask for latest * Use latest propshaft Co-authored-by: Hartley McGuire Co-authored-by: Richard Macklin <1863540+rmacklin@users.noreply.github.com> --- Gemfile | 2 ++ Gemfile.lock | 21 ++++++++------- .../action_view/helpers/asset_url_helper.rb | 4 +-- rails.gemspec | 3 +-- railties/lib/rails/all.rb | 1 - railties/lib/rails/app_updater.rb | 2 +- railties/lib/rails/generators/app_base.rb | 26 ++++++++++++++++--- .../generators/rails/app/app_generator.rb | 14 ++++++---- .../generators/rails/app/templates/Gemfile.tt | 2 +- .../app/templates/config/application.rb.tt | 1 - .../config/environments/development.rb.tt | 2 +- .../config/environments/production.rb.tt | 2 +- .../rails/plugin/plugin_generator.rb | 2 +- .../rails/plugin/templates/bin/rails.tt | 1 - .../test/generators/app_generator_test.rb | 7 +++-- .../test/generators/generators_test_helper.rb | 1 + .../test/generators/plugin_generator_test.rb | 3 +-- .../test/generators/shared_generator_tests.rb | 10 +++---- railties/test/isolation/abstract_unit.rb | 2 +- railties/test/railties/engine_test.rb | 1 - 20 files changed, 63 insertions(+), 44 deletions(-) diff --git a/Gemfile b/Gemfile index 9e1e448d64a..05cbb74e19b 100644 --- a/Gemfile +++ b/Gemfile @@ -9,6 +9,8 @@ gemspec # We need a newish Rake since Active Job sets its test tasks' descriptions. gem "rake", ">= 11.1" +gem "sprockets-rails", ">= 2.0.0" +gem "propshaft", ">= 0.1.7" gem "capybara", ">= 3.26" gem "selenium-webdriver", ">= 4.0.0.alpha7" diff --git a/Gemfile.lock b/Gemfile.lock index bf12648d1f1..8ef9d35fe48 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -96,7 +96,6 @@ PATH activesupport (= 7.0.0.alpha2) bundler (>= 1.15.0) railties (= 7.0.0.alpha2) - sprockets-rails (>= 2.0.0) railties (7.0.0.alpha2) actionpack (= 7.0.0.alpha2) activesupport (= 7.0.0.alpha2) @@ -184,8 +183,8 @@ GEM crack (0.4.5) rexml crass (1.0.6) - cssbundling-rails (0.1.0) - rails (>= 6.0.0) + cssbundling-rails (0.2.2) + railties (>= 6.0.0) curses (1.4.2) daemons (1.4.0) dalli (2.7.11) @@ -291,14 +290,14 @@ GEM image_processing (1.12.1) mini_magick (>= 4.9.5, < 5) ruby-vips (>= 2.0.17, < 3) - importmap-rails (0.5.1) + importmap-rails (0.7.3) rails (>= 6.0.0) io-console (0.5.9) irb (1.3.7) reline (>= 0.2.7) jmespath (1.4.0) - jsbundling-rails (0.1.0) - rails (>= 6.0.0) + jsbundling-rails (0.1.7) + railties (>= 6.0.0) json (2.5.1) jwt (2.2.3) kindlerb (1.2.0) @@ -352,6 +351,8 @@ GEM ast (~> 2.4.1) path_expander (1.1.0) pg (1.2.3) + propshaft (0.1.7) + rails (>= 7.0.0.alpha2) psych (3.3.2) public_suffix (4.0.6) puma (5.3.2) @@ -476,7 +477,7 @@ GEM sprockets (>= 3.0.0) sqlite3 (1.4.2) stackprof (0.2.17) - stimulus-rails (0.4.2) + stimulus-rails (0.5.4) rails (>= 6.0.0) sucker_punch (3.0.1) concurrent-ruby (~> 1.0) @@ -491,7 +492,7 @@ GEM thor (1.1.0) tilt (2.0.10) trailblazer-option (0.1.1) - turbo-rails (0.7.11) + turbo-rails (0.7.14) rails (>= 6.0.0) tzinfo (2.0.4) concurrent-ruby (~> 1.0) @@ -561,6 +562,7 @@ DEPENDENCIES mysql2 (~> 0.5)! nokogiri (>= 1.8.1, != 1.11.0) pg (~> 1.1) + propshaft (>= 0.1.7) psych (~> 3.0) puma que @@ -587,6 +589,7 @@ DEPENDENCIES sidekiq sneakers sprockets-export + sprockets-rails (>= 2.0.0) sqlite3 (~> 1.4) stackprof stimulus-rails @@ -603,4 +606,4 @@ DEPENDENCIES websocket-client-simple! BUNDLED WITH - 2.2.25 + 2.2.28 diff --git a/actionview/lib/action_view/helpers/asset_url_helper.rb b/actionview/lib/action_view/helpers/asset_url_helper.rb index 7653f2a118a..90218e32972 100644 --- a/actionview/lib/action_view/helpers/asset_url_helper.rb +++ b/actionview/lib/action_view/helpers/asset_url_helper.rb @@ -121,7 +121,7 @@ module ActionView URI_REGEXP = %r{^[-a-z]+://|^(?:cid|data):|^//}i # This is the entry point for all assets. - # When using the asset pipeline (i.e. sprockets and sprockets-rails), the + # When using an asset pipeline gem (e.g. propshaft or sprockets-rails), the # behavior is "enhanced". You can bypass the asset pipeline by passing in # skip_pipeline: true to the options. # @@ -130,7 +130,7 @@ module ActionView # === With the asset pipeline # # All options passed to +asset_path+ will be passed to +compute_asset_path+ - # which is implemented by sprockets-rails. + # which is implemented by asset pipeline gems. # # asset_path("application.js") # => "/assets/application-60aa4fdc5cea14baf5400fba1abf4f2a46a5166bad4772b1effe341570f07de9.js" # asset_path('application.js', host: 'example.com') # => "//example.com/assets/application.js" diff --git a/rails.gemspec b/rails.gemspec index 3bb12a61d97..9cec2c2c70c 100644 --- a/rails.gemspec +++ b/rails.gemspec @@ -41,6 +41,5 @@ Gem::Specification.new do |s| s.add_dependency "actiontext", version s.add_dependency "railties", version - s.add_dependency "bundler", ">= 1.15.0" - s.add_dependency "sprockets-rails", ">= 2.0.0" + s.add_dependency "bundler", ">= 1.15.0" end diff --git a/railties/lib/rails/all.rb b/railties/lib/rails/all.rb index da810f1eed6..e620dae1bf7 100644 --- a/railties/lib/rails/all.rb +++ b/railties/lib/rails/all.rb @@ -15,7 +15,6 @@ require "rails" action_mailbox/engine action_text/engine rails/test_unit/railtie - sprockets/railtie ).each do |railtie| begin require railtie diff --git a/railties/lib/rails/app_updater.rb b/railties/lib/rails/app_updater.rb index e93f769c187..206faa6ad30 100644 --- a/railties/lib/rails/app_updater.rb +++ b/railties/lib/rails/app_updater.rb @@ -25,7 +25,7 @@ module Rails options[:skip_active_storage] = !defined?(ActiveStorage::Engine) || !defined?(ActiveRecord::Railtie) options[:skip_action_mailer] = !defined?(ActionMailer::Railtie) options[:skip_action_cable] = !defined?(ActionCable::Engine) - options[:skip_sprockets] = !defined?(Sprockets::Railtie) + options[:skip_asset_pipeline] = !defined?(Sprockets::Railtie) && !defined?(Propshaft::Railtie) options[:skip_bootsnap] = !defined?(Bootsnap) options[:updating] = true options diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 3b149cb9918..20796a96ab8 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -58,8 +58,10 @@ module Rails class_option :skip_action_cable, type: :boolean, aliases: "-C", default: false, desc: "Skip Action Cable files" - class_option :skip_sprockets, type: :boolean, aliases: "-S", default: false, - desc: "Skip Sprockets files" + class_option :skip_asset_pipeline, type: :boolean, aliases: "-A", default: false + + class_option :asset_pipeline, type: :string, aliases: "-a", default: "sprockets", + desc: "Choose your asset pipeline [options: sprockets (default), propshaft]" class_option :skip_javascript, type: :boolean, aliases: "-J", default: name == "plugin", desc: "Skip JavaScript files" @@ -106,6 +108,7 @@ module Rails private def gemfile_entries # :doc: [rails_gemfile_entry, + asset_pipeline_gemfile_entry, database_gemfile_entry, web_server_gemfile_entry, javascript_gemfile_entry, @@ -165,13 +168,25 @@ module Rails GemfileEntry.new "puma", "~> 5.0", "Use the Puma web server [https://github.com/puma/puma]" end + def asset_pipeline_gemfile_entry + return [] if options[:skip_asset_pipeline] + + if options[:asset_pipeline] == "sprockets" + GemfileEntry.version "sprockets-rails", ">= 2.0.0", + "The traditional bundling and transpiling asset pipeline for Rails." + elsif options[:asset_pipeline] == "propshaft" + GemfileEntry.version "propshaft", ">= 0.1.7", "The modern asset pipeline for Rails." + else + [] + end + end + def include_all_railties? # :doc: [ options.values_at( :skip_active_record, :skip_action_mailer, :skip_test, - :skip_sprockets, :skip_action_cable, :skip_active_job ), @@ -218,6 +233,11 @@ module Rails options[:skip_dev_gems] end + def skip_sprockets? + options[:skip_asset_pipeline] || options[:asset_pipeline] != "sprockets" + end + + class GemfileEntry < Struct.new(:name, :version, :comment, :options, :commented_out) def initialize(name, version, comment, options = {}, commented_out = false) super diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index b85fe336a2b..6f01c87ec7b 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -138,15 +138,15 @@ module Rails template "config/storage.yml" end - if options[:skip_sprockets] && !assets_config_exist + if skip_sprockets? && !assets_config_exist remove_file "config/initializers/assets.rb" end - if options[:skip_sprockets] && !asset_manifest_exist + if skip_sprockets? && !asset_manifest_exist remove_file "app/assets/config/manifest.js" end - if options[:skip_sprockets] && !asset_app_stylesheet_exist + if skip_sprockets? && !asset_app_stylesheet_exist remove_file "app/assets/stylesheets/application.css" end @@ -163,6 +163,10 @@ module Rails remove_file "config/initializers/permissions_policy.rb" end end + + if !skip_sprockets? + insert_into_file "config/application.rb", %(require "sprockets/railtie"), after: /require\(["']rails\/all["']\)\n/ + end end def master_key @@ -275,7 +279,7 @@ module Rails # Force sprockets and JavaScript to be skipped when generating API only apps. # Can't modify options hash as it's frozen by default. if options[:api] - self.options = options.merge(skip_sprockets: true, skip_javascript: true).freeze + self.options = options.merge(skip_asset_pipeline: true, skip_javascript: true).freeze end if options[:minimal] @@ -440,7 +444,7 @@ module Rails end def delete_assets_initializer_skipping_sprockets - if options[:skip_sprockets] + if skip_sprockets? remove_file "config/initializers/assets.rb" remove_file "app/assets/config/manifest.js" remove_file "app/assets/stylesheets/application.css" diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt index f99f91c7515..c07997031c3 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt @@ -26,7 +26,7 @@ gem "tzinfo-data", platforms: %i[ mingw mswin x64_mingw jruby ] # Reduces boot times through caching; required in config/boot.rb gem "bootsnap", ">= 1.4.4", require: false <% end -%> -<% unless options.skip_sprockets? || options.minimal? -%> +<% unless skip_sprockets? || options.minimal? -%> # Use Sass to process CSS # gem "sassc-rails", "~> 2.1" diff --git a/railties/lib/rails/generators/rails/app/templates/config/application.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/application.rb.tt index 12b7d8b05f6..aefca77a711 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/application.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/application.rb.tt @@ -15,7 +15,6 @@ require "action_controller/railtie" <%= comment_if :skip_action_text %>require "action_text/engine" require "action_view/railtie" <%= comment_if :skip_action_cable %>require "action_cable/engine" -<%= comment_if :skip_sprockets %>require "sprockets/railtie" <%= comment_if :skip_test %>require "rails/test_unit/railtie" <% end -%> diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt index fa1bd04579b..38430f72b4c 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt @@ -64,7 +64,7 @@ Rails.application.configure do config.active_record.verbose_query_logs = true <%- end -%> - <%- unless options.skip_sprockets? -%> + <%- unless skip_sprockets? -%> # Suppress logger output for asset requests. config.assets.quiet = true <%- end -%> diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt index 2bc3486a91b..21c1a1ced1b 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt @@ -26,7 +26,7 @@ Rails.application.configure do # Apache or NGINX already handles this. config.public_file_server.enabled = ENV["RAILS_SERVE_STATIC_FILES"].present? - <%- unless options.skip_sprockets? -%> + <%- unless skip_sprockets? -%> # Compress CSS using a preprocessor. # config.assets.css_compressor = :sass diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index 8a86f97a69e..9efcbe79911 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -316,7 +316,7 @@ module Rails mute do build(:generate_test_dummy) build(:test_dummy_config) - build(:test_dummy_sprocket_assets) unless options[:skip_sprockets] + build(:test_dummy_sprocket_assets) unless skip_sprockets? build(:test_dummy_clean) # ensure that bin/rails has proper dummy_path build(:bin, true) diff --git a/railties/lib/rails/generators/rails/plugin/templates/bin/rails.tt b/railties/lib/rails/generators/rails/plugin/templates/bin/rails.tt index d1eaf1fd4e2..5a9fedf2b5f 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/bin/rails.tt +++ b/railties/lib/rails/generators/rails/plugin/templates/bin/rails.tt @@ -24,7 +24,6 @@ require "action_controller/railtie" <%= comment_if :skip_action_mailer %>require "action_mailer/railtie" require "action_view/railtie" <%= comment_if :skip_action_cable %>require "action_cable/engine" -<%= comment_if :skip_sprockets %>require "sprockets/railtie" <%= comment_if :skip_test %>require "rails/test_unit/railtie" <% end -%> require "rails/engine/commands" diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 2283ace0391..86af490630c 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -230,12 +230,12 @@ class AppGeneratorTest < Rails::Generators::TestCase end end - def test_app_update_does_not_generate_assets_initializer_when_skip_sprockets_is_given + def test_app_update_does_not_generate_assets_initializer_when_sprockets_is_not_used app_root = File.join(destination_root, "myapp") - run_generator [app_root, "--skip-sprockets"] + run_generator [app_root, "-a", "none"] stub_rails_application(app_root) do - generator = Rails::Generators::AppGenerator.new ["rails"], { update: true, skip_sprockets: true }, { destination_root: app_root, shell: @shell } + generator = Rails::Generators::AppGenerator.new ["rails"], { update: true, asset_pipeline: "none" }, { destination_root: app_root, shell: @shell } generator.send(:app_const) quietly { generator.update_config_files } @@ -1005,7 +1005,6 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_match(/#\s+require\s+["']action_mailbox\/engine["']/, content) assert_match(/#\s+require\s+["']action_text\/engine["']/, content) assert_match(/#\s+require\s+["']action_cable\/engine["']/, content) - assert_match(/\s+require\s+["']sprockets\/railtie["']/, content) end assert_no_gem "jbuilder", app_root diff --git a/railties/test/generators/generators_test_helper.rb b/railties/test/generators/generators_test_helper.rb index a1fe893c0af..9f9cf71f7be 100644 --- a/railties/test/generators/generators_test_helper.rb +++ b/railties/test/generators/generators_test_helper.rb @@ -96,6 +96,7 @@ module GeneratorsTestHelper depend_on_bootsnap: false, depends_on_system_test: false, options: ActiveSupport::OrderedOptions.new, + skip_sprockets: false, } end end diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 587dc9047c3..bfe48d9913f 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -121,7 +121,7 @@ class PluginGeneratorTest < Rails::Generators::TestCase end def test_generating_in_full_mode_with_almost_of_all_skip_options - run_generator [destination_root, "--full", "-M", "-O", "-C", "-S", "-T", "--skip-active-storage"] + run_generator [destination_root, "--full", "-M", "-O", "-C", "-T", "--skip-active-storage"] assert_file "bin/rails" do |content| assert_no_match(/\s+require\s+["']rails\/all["']/, content) end @@ -129,7 +129,6 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_file "bin/rails", /#\s+require\s+["']active_storage\/engine["']/ assert_file "bin/rails", /#\s+require\s+["']action_mailer\/railtie["']/ assert_file "bin/rails", /#\s+require\s+["']action_cable\/engine["']/ - assert_file "bin/rails", /#\s+require\s+["']sprockets\/railtie["']/ assert_file "bin/rails", /#\s+require\s+["']rails\/test_unit\/railtie["']/ end diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index fe0949e9e42..d5b02f80b7d 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -119,8 +119,7 @@ module SharedGeneratorTests "--skip-action-mailer", "--skip-action-mailbox", "--skip-action-text", - "--skip-action-cable", - "--skip-sprockets" + "--skip-action-cable" ] assert_file "#{application_path}/config/application.rb", /^require\s+["']rails["']/ @@ -136,7 +135,6 @@ module SharedGeneratorTests end assert_file "#{application_path}/config/application.rb", /^require\s+["']action_view\/railtie["']/ assert_file "#{application_path}/config/application.rb", /^# require\s+["']action_cable\/engine["']/ - assert_file "#{application_path}/config/application.rb", /^# require\s+["']sprockets\/railtie["']/ assert_file "#{application_path}/config/application.rb", /^require\s+["']rails\/test_unit\/railtie["']/ end @@ -292,15 +290,13 @@ module SharedGeneratorTests end end - def test_generator_if_skip_sprockets_is_given - run_generator [destination_root, "--skip-sprockets"] + def test_generator_when_sprockets_is_not_used + run_generator [destination_root, "-a", "none"] assert_no_file "#{application_path}/config/initializers/assets.rb" assert_no_file "#{application_path}/app/assets/config/manifest.js" assert_no_file "#{application_path}/app/assets/stylesheets/application.css" - assert_file "#{application_path}/config/application.rb", /#\s+require\s+["']sprockets\/railtie["']/ - assert_file "Gemfile" do |content| assert_no_match(/sass-rails/, content) end diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 0785d545e75..063452c26f2 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -535,7 +535,7 @@ Module.new do # Fake 'Bundler.require' -- we run using the repo's Gemfile, not an # app-specific one: we don't want to require every gem that lists. contents = File.read("#{app_template_path}/config/application.rb") - contents.sub!(/^Bundler\.require.*/, "%w(importmap-rails).each { |r| require r }") + contents.sub!(/^Bundler\.require.*/, "%w(sprockets/railtie importmap-rails).each { |r| require r }") File.write("#{app_template_path}/config/application.rb", contents) require "rails" diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index 432eed4e67f..0af81ba3992 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -1670,7 +1670,6 @@ en: require "action_controller/railtie" require "action_mailer/railtie" require "action_view/railtie" - require "sprockets/railtie" require "rails/test_unit/railtie" RUBY environment = File.read("#{app_path}/config/application.rb")