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 <skipkayhil@gmail.com>
Co-authored-by: Richard Macklin <1863540+rmacklin@users.noreply.github.com>
This commit is contained in:
David Heinemeier Hansson 2021-10-09 17:03:05 +02:00 committed by GitHub
parent 2ec2ee6899
commit fb1ab3460a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 63 additions and 44 deletions

@ -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"

@ -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

@ -121,7 +121,7 @@ module AssetUrlHelper
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
# <tt>skip_pipeline: true</tt> to the options.
#
@ -130,7 +130,7 @@ module AssetUrlHelper
# === 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"

@ -41,6 +41,5 @@
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

@ -15,7 +15,6 @@
action_mailbox/engine
action_text/engine
rails/test_unit/railtie
sprockets/railtie
).each do |railtie|
begin
require railtie

@ -25,7 +25,7 @@ def generator_options
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

@ -58,8 +58,10 @@ def self.add_shared_options_for(name)
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 @@ def initialize(*)
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 @@ def web_server_gemfile_entry # :doc:
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 @@ def skip_dev_gems? # :doc:
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

@ -138,15 +138,15 @@ def config_when_updating
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 @@ def config_when_updating
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 @@ def initialize(*args)
# 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 @@ def delete_public_files_if_api_option
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"

@ -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"

@ -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 -%>

@ -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 -%>

@ -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

@ -316,7 +316,7 @@ def create_dummy_app(path = nil)
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)

@ -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"

@ -230,12 +230,12 @@ def test_app_update_does_not_remove_rack_cors_if_already_present
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 @@ def test_minimal_rails_app
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

@ -96,6 +96,7 @@ def gemfile_locals
depend_on_bootsnap: false,
depends_on_system_test: false,
options: ActiveSupport::OrderedOptions.new,
skip_sprockets: false,
}
end
end

@ -121,7 +121,7 @@ def test_version_control_initializes_git_repo_with_user_default_branch
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 @@ def test_generating_in_full_mode_with_almost_of_all_skip_options
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

@ -119,8 +119,7 @@ def test_default_frameworks_are_required_when_others_are_removed
"--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 @@ def test_default_frameworks_are_required_when_others_are_removed
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 @@ def test_generator_if_skip_action_cable_is_given
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

@ -535,7 +535,7 @@ def self.sh(cmd)
# 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"

@ -1670,7 +1670,6 @@ def restrict_frameworks
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")