From 51e99fb61535163410e448964e7c1aff9b35f80e Mon Sep 17 00:00:00 2001 From: Simon Fish Date: Sun, 8 Sep 2024 19:34:33 +0100 Subject: [PATCH] Make accommodations for component-local config Introduces a `ViewComponent::GlobalConfig` object that will be the source for global configuration going forward. Ideally in the future, this will only house options that universally affect ViewComponent regardless of whether components are sourced from an engine or not (e.g. enabling the capture compatibility patch), and more options can move to a component-local config. For these options, classes inheriting from `ViewComponent::Base` will want to override configuration themselves. This was initially written to support extracting the incoming strict_helpers_enabled? option, but applies to everything. --- Rakefile | 2 +- .../concerns/view_component/preview_actions.rb | 6 +++--- app/helpers/preview_helper.rb | 4 ++-- app/views/view_components/preview.html.erb | 4 ++-- docs/CHANGELOG.md | 4 ++++ lib/rails/generators/abstract_generator.rb | 8 ++++---- .../component/component_generator.rb | 8 ++++---- .../generators/locale/component_generator.rb | 2 +- .../generators/preview/component_generator.rb | 4 ++-- .../generators/rspec/component_generator.rb | 2 +- lib/view_component.rb | 1 + lib/view_component/base.rb | 8 ++++---- lib/view_component/config.rb | 2 +- lib/view_component/engine.rb | 8 ++++---- lib/view_component/global_config.rb | 3 +++ lib/view_component/instrumentation.rb | 2 +- lib/view_component/preview.rb | 2 +- .../rails/tasks/view_component.rake | 4 ++-- lib/view_component/test_helpers.rb | 2 +- lib/view_component/use_helpers.rb | 4 +++- test/sandbox/test/integration_test.rb | 18 +++++++++++------- test/sandbox/test/rendering_test.rb | 4 ++-- test/test_engine/test_helper.rb | 2 +- 23 files changed, 59 insertions(+), 45 deletions(-) create mode 100644 lib/view_component/global_config.rb diff --git a/Rakefile b/Rakefile index 212b5810e..2531f71e8 100644 --- a/Rakefile +++ b/Rakefile @@ -95,7 +95,7 @@ namespace :docs do require "rails" require "action_controller" require "view_component" - ViewComponent::Base.config.view_component_path = "view_component" + ViewComponent::GlobalConfig.view_component_path = "view_component" require "view_component/docs_builder_component" error_keys = registry.keys.select { |key| key.to_s.include?("Error::MESSAGE") }.map(&:to_s) diff --git a/app/controllers/concerns/view_component/preview_actions.rb b/app/controllers/concerns/view_component/preview_actions.rb index 1a4e7536e..b7b981cdf 100644 --- a/app/controllers/concerns/view_component/preview_actions.rb +++ b/app/controllers/concerns/view_component/preview_actions.rb @@ -54,12 +54,12 @@ def previews # :doc: def default_preview_layout - ViewComponent::Base.config.default_preview_layout + GlobalConfig.default_preview_layout end # :doc: def show_previews? - ViewComponent::Base.config.show_previews + GlobalConfig.show_previews end # :doc: @@ -102,7 +102,7 @@ def prepend_application_view_paths end def prepend_preview_examples_view_path - prepend_view_path(ViewComponent::Base.preview_paths) + prepend_view_path(GlobalConfig.preview_paths) end end end diff --git a/app/helpers/preview_helper.rb b/app/helpers/preview_helper.rb index bac7557bc..f8e200bea 100644 --- a/app/helpers/preview_helper.rb +++ b/app/helpers/preview_helper.rb @@ -35,7 +35,7 @@ def find_template_data_for_preview_source(lookup_context:, template_identifier:) # Fetch template source via finding it through preview paths # to accomodate source view when exclusively using templates # for previews for Rails < 6.1. - all_template_paths = ViewComponent::Base.config.preview_paths.map do |preview_path| + all_template_paths = ViewComponent::GlobalConfig.preview_paths.map do |preview_path| Dir.glob("#{preview_path}/**/*") end.flatten @@ -80,6 +80,6 @@ def prism_language_name_by_template_path(template_file_path:) # :nocov: def serve_static_preview_assets? - ViewComponent::Base.config.show_previews && Rails.application.config.public_file_server.enabled + ViewComponent::GlobalConfig.show_previews && Rails.application.config.public_file_server.enabled end end diff --git a/app/views/view_components/preview.html.erb b/app/views/view_components/preview.html.erb index f364fd725..3101fb00e 100644 --- a/app/views/view_components/preview.html.erb +++ b/app/views/view_components/preview.html.erb @@ -1,5 +1,5 @@ <% if @render_args[:component] %> - <% if ViewComponent::Base.config.render_monkey_patch_enabled || Rails.version.to_f >= 6.1 %> + <% if ViewComponent::GlobalConfig.render_monkey_patch_enabled || Rails.version.to_f >= 6.1 %> <%= render(@render_args[:component], @render_args[:args], &@render_args[:block]) %> <% else %> <%= render_component(@render_args[:component], &@render_args[:block]) %> @@ -8,6 +8,6 @@ <%= render template: @render_args[:template], locals: @render_args[:locals] || {} %> <% end %> -<% if ViewComponent::Base.config.show_previews_source %> +<% if ViewComponent::GlobalConfig.show_previews_source %> <%= preview_source %> <% end %> diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 6ae2a2a32..277c2a964 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,6 +10,10 @@ nav_order: 5 ## main +* Make accommodations for component-local config to be introduced in future. + + *Simon Fish* + * Remove JS and CSS docs as they proved difficult to maintain and lacked consensus. *Joel Hawksley* diff --git a/lib/rails/generators/abstract_generator.rb b/lib/rails/generators/abstract_generator.rb index 8159f4e64..11eed4474 100644 --- a/lib/rails/generators/abstract_generator.rb +++ b/lib/rails/generators/abstract_generator.rb @@ -29,7 +29,7 @@ def file_name end def component_path - ViewComponent::Base.config.view_component_path + GlobalConfig.view_component_path end def stimulus_controller @@ -42,15 +42,15 @@ def stimulus_controller end def sidecar? - options["sidecar"] || ViewComponent::Base.config.generate.sidecar + options["sidecar"] || GlobalConfig.generate.sidecar end def stimulus? - options["stimulus"] || ViewComponent::Base.config.generate.stimulus_controller + options["stimulus"] || GlobalConfig.generate.stimulus_controller end def typescript? - options["typescript"] || ViewComponent::Base.config.generate.typescript + options["typescript"] || GlobalConfig.generate.typescript end end end diff --git a/lib/rails/generators/component/component_generator.rb b/lib/rails/generators/component/component_generator.rb index d5e4b8b3c..79825ecc7 100644 --- a/lib/rails/generators/component/component_generator.rb +++ b/lib/rails/generators/component/component_generator.rb @@ -13,12 +13,12 @@ class ComponentGenerator < Rails::Generators::NamedBase check_class_collision suffix: "Component" class_option :inline, type: :boolean, default: false - class_option :locale, type: :boolean, default: ViewComponent::Base.config.generate.locale + class_option :locale, type: :boolean, default: ViewComponent::GlobalConfig.generate.locale class_option :parent, type: :string, desc: "The parent class for the generated component" - class_option :preview, type: :boolean, default: ViewComponent::Base.config.generate.preview + class_option :preview, type: :boolean, default: ViewComponent::GlobalConfig.generate.preview class_option :sidecar, type: :boolean, default: false class_option :stimulus, type: :boolean, - default: ViewComponent::Base.config.generate.stimulus_controller + default: ViewComponent::GlobalConfig.generate.stimulus_controller class_option :skip_suffix, type: :boolean, default: false def create_component_file @@ -42,7 +42,7 @@ def create_component_file def parent_class return options[:parent] if options[:parent] - ViewComponent::Base.config.component_parent_class || default_parent_class + ViewComponent::GlobalConfig.component_parent_class || default_parent_class end def initialize_signature diff --git a/lib/rails/generators/locale/component_generator.rb b/lib/rails/generators/locale/component_generator.rb index 55f735353..de7193361 100644 --- a/lib/rails/generators/locale/component_generator.rb +++ b/lib/rails/generators/locale/component_generator.rb @@ -12,7 +12,7 @@ class ComponentGenerator < ::Rails::Generators::NamedBase class_option :sidecar, type: :boolean, default: false def create_locale_file - if ViewComponent::Base.config.generate.distinct_locale_files + if ViewComponent::GlobalConfig.generate.distinct_locale_files I18n.available_locales.each do |locale| create_file destination(locale), translations_hash([locale]).to_yaml end diff --git a/lib/rails/generators/preview/component_generator.rb b/lib/rails/generators/preview/component_generator.rb index abb6fbd07..32886421d 100644 --- a/lib/rails/generators/preview/component_generator.rb +++ b/lib/rails/generators/preview/component_generator.rb @@ -4,13 +4,13 @@ module Preview module Generators class ComponentGenerator < ::Rails::Generators::NamedBase source_root File.expand_path("templates", __dir__) - class_option :preview_path, type: :string, desc: "Path for previews, required when multiple preview paths are configured", default: ViewComponent::Base.config.generate.preview_path + class_option :preview_path, type: :string, desc: "Path for previews, required when multiple preview paths are configured", default: ViewComponent::GlobalConfig.generate.preview_path argument :attributes, type: :array, default: [], banner: "attribute" check_class_collision suffix: "ComponentPreview" def create_preview_file - preview_paths = ViewComponent::Base.config.preview_paths + preview_paths = ViewComponent::GlobalConfig.preview_paths optional_path = options[:preview_path] return if preview_paths.count > 1 && optional_path.blank? diff --git a/lib/rails/generators/rspec/component_generator.rb b/lib/rails/generators/rspec/component_generator.rb index 605ada5fb..c804bd20a 100644 --- a/lib/rails/generators/rspec/component_generator.rb +++ b/lib/rails/generators/rspec/component_generator.rb @@ -16,7 +16,7 @@ def create_test_file private def spec_component_path - return "spec/components" unless ViewComponent::Base.config.generate.use_component_path_for_rspec_tests + return "spec/components" unless ViewComponent::GlobalConfig.generate.use_component_path_for_rspec_tests configured_component_path = component_path if configured_component_path.start_with?("app#{File::SEPARATOR}") diff --git a/lib/view_component.rb b/lib/view_component.rb index 62c9cd2cb..df6b8df2b 100644 --- a/lib/view_component.rb +++ b/lib/view_component.rb @@ -13,6 +13,7 @@ module ViewComponent autoload :ComponentError autoload :Config autoload :Deprecation + autoload :GlobalConfig autoload :InlineTemplate autoload :Instrumentation autoload :Preview diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 2cb42a545..d5e3b0409 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "action_view" -require "active_support/configurable" require "view_component/collection" require "view_component/compile_cache" require "view_component/compiler" @@ -25,7 +24,7 @@ class << self # # @return [ActiveSupport::OrderedOptions] def config - ViewComponent::Config.current + GlobalConfig end end @@ -48,6 +47,8 @@ def config class_attribute :__vc_strip_trailing_whitespace, instance_accessor: false, instance_predicate: false self.__vc_strip_trailing_whitespace = false # class_attribute:default doesn't work until Rails 5.2 + delegate :component_config, to: :class + attr_accessor :__vc_original_view_context # Components render in their own view context. Helpers and other functionality @@ -227,7 +228,6 @@ def controller # @return [ActionView::Base] def helpers raise HelpersCalledBeforeRenderError if view_context.nil? - # Attempt to re-use the original view_context passed to the first # component rendered in the rendering pipeline. This prevents the # instantiation of a new view_context via `controller.view_context` which @@ -550,7 +550,7 @@ def render_template_for(variant = nil, format = nil) # If Rails application is loaded, removes the first part of the path and the extension. if defined?(Rails) && Rails.application child.virtual_path = child.identifier.gsub( - /(.*#{Regexp.quote(ViewComponent::Base.config.view_component_path)})|(\.rb)/, "" + /(.*#{Regexp.quote(GlobalConfig.view_component_path)})|(\.rb)/, "" ) end diff --git a/lib/view_component/config.rb b/lib/view_component/config.rb index 3edcd22d2..48d75535d 100644 --- a/lib/view_component/config.rb +++ b/lib/view_component/config.rb @@ -11,7 +11,7 @@ class << self alias_method :default, :new def defaults - ActiveSupport::OrderedOptions.new.merge!({ + ActiveSupport::InheritableOptions.new({ generate: default_generate_options, preview_controller: "ViewComponentsController", preview_route: "/rails/view_components", diff --git a/lib/view_component/engine.rb b/lib/view_component/engine.rb index b920eb5d8..31ca11a66 100644 --- a/lib/view_component/engine.rb +++ b/lib/view_component/engine.rb @@ -6,7 +6,7 @@ module ViewComponent class Engine < Rails::Engine # :nodoc: - config.view_component = ViewComponent::Config.current + config.view_component = ViewComponent::Config.defaults if Rails.version.to_f < 8.0 rake_tasks do @@ -16,8 +16,8 @@ class Engine < Rails::Engine # :nodoc: initializer "view_component.stats_directories" do |app| require "rails/code_statistics" - if Rails.root.join(ViewComponent::Base.view_component_path).directory? - Rails::CodeStatistics.register_directory("ViewComponents", ViewComponent::Base.view_component_path) + if Rails.root.join(GlobalConfig.view_component_path).directory? + Rails::CodeStatistics.register_directory("ViewComponents", GlobalConfig.view_component_path) end if Rails.root.join("test/components").directory? @@ -29,7 +29,7 @@ class Engine < Rails::Engine # :nodoc: initializer "view_component.set_configs" do |app| options = app.config.view_component - %i[generate preview_controller preview_route show_previews_source].each do |config_option| + %i[generate preview_controller preview_route].each do |config_option| options[config_option] ||= ViewComponent::Base.public_send(config_option) end options.instrumentation_enabled = false if options.instrumentation_enabled.nil? diff --git a/lib/view_component/global_config.rb b/lib/view_component/global_config.rb new file mode 100644 index 000000000..563a2a396 --- /dev/null +++ b/lib/view_component/global_config.rb @@ -0,0 +1,3 @@ +module ViewComponent + GlobalConfig = (defined?(Rails) && Rails.application) ? Rails.application.config.view_component : Config.defaults # standard:disable Naming/ConstantName +end diff --git a/lib/view_component/instrumentation.rb b/lib/view_component/instrumentation.rb index d63efd283..1d95be28b 100644 --- a/lib/view_component/instrumentation.rb +++ b/lib/view_component/instrumentation.rb @@ -23,7 +23,7 @@ def render_in(view_context, &block) private def notification_name - return "!render.view_component" if ViewComponent::Base.config.use_deprecated_instrumentation_name + return "!render.view_component" if GlobalConfig.use_deprecated_instrumentation_name "render.view_component" end diff --git a/lib/view_component/preview.rb b/lib/view_component/preview.rb index 2d3c84465..ef962c918 100644 --- a/lib/view_component/preview.rb +++ b/lib/view_component/preview.rb @@ -109,7 +109,7 @@ def load_previews private def preview_paths - Base.preview_paths + ViewComponent::GlobalConfig.preview_paths end end end diff --git a/lib/view_component/rails/tasks/view_component.rake b/lib/view_component/rails/tasks/view_component.rake index ae8662cd1..85f59145f 100644 --- a/lib/view_component/rails/tasks/view_component.rake +++ b/lib/view_component/rails/tasks/view_component.rake @@ -7,8 +7,8 @@ namespace :view_component do # :nocov: require "rails/code_statistics" - if Rails.root.join(ViewComponent::Base.view_component_path).directory? - ::STATS_DIRECTORIES << ["ViewComponents", ViewComponent::Base.view_component_path] + if Rails.root.join(ViewComponent::GlobalConfig.view_component_path).directory? + ::STATS_DIRECTORIES << ["ViewComponents", ViewComponent::GlobalConfig.view_component_path] end if Rails.root.join("test/components").directory? diff --git a/lib/view_component/test_helpers.rb b/lib/view_component/test_helpers.rb index 3677763fb..bd8e8ee48 100644 --- a/lib/view_component/test_helpers.rb +++ b/lib/view_component/test_helpers.rb @@ -250,7 +250,7 @@ def with_request_url(full_path, host: nil, method: nil, format: ViewComponent::B # # @return [ActionController::Base] def vc_test_controller - @vc_test_controller ||= __vc_test_helpers_build_controller(Base.test_controller.constantize) + @vc_test_controller ||= __vc_test_helpers_build_controller(GlobalConfig.test_controller.constantize) end # Access the request used by `render_inline`: diff --git a/lib/view_component/use_helpers.rb b/lib/view_component/use_helpers.rb index 8d395da26..c0a324de5 100644 --- a/lib/view_component/use_helpers.rb +++ b/lib/view_component/use_helpers.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "view_component/errors" + module ViewComponent::UseHelpers extend ActiveSupport::Concern @@ -13,7 +15,7 @@ def use_helper(helper_method, from: nil, prefix: false) class_eval(<<-RUBY, __FILE__, __LINE__ + 1) def #{helper_method_name}(*args, &block) - raise HelpersCalledBeforeRenderError if view_context.nil? + raise ::ViewComponent::HelpersCalledBeforeRenderError if view_context.nil? #{define_helper(helper_method: helper_method, source: from)} end diff --git a/test/sandbox/test/integration_test.rb b/test/sandbox/test/integration_test.rb index 76e65cf4d..4f12a0feb 100644 --- a/test/sandbox/test/integration_test.rb +++ b/test/sandbox/test/integration_test.rb @@ -579,7 +579,7 @@ def test_render_component_in_turbo_stream expected_response_body = <<~TURBOSTREAM TURBOSTREAM - if ViewComponent::Base.config.capture_compatibility_patch_enabled + if ViewComponent::GlobalConfig.capture_compatibility_patch_enabled assert_equal expected_response_body, response.body else assert_not_equal expected_response_body, response.body @@ -716,10 +716,14 @@ def test_cached_partial Rails.cache.clear end - def test_config_options_shared_between_base_and_engine - config_entrypoints = [Rails.application.config.view_component, ViewComponent::Base.config] - 2.times do - config_entrypoints.first.yield_self do |config| + def test_globalconfig_is_proxy_for_rails_app_config + config_entrypoints = [ + ["Rails application config", Rails.application.config.view_component], + ["Global config", ViewComponent::GlobalConfig], + ["ViewComponent::Base's config", ViewComponent::Base.config] + ] + config_entrypoints.permutation(2) do |(first_entrypoint_name, first_entrypoint), (second_entrypoint_name, second_entrypoint)| + first_entrypoint.yield_self do |config| { generate: config.generate.dup.tap { |c| c.sidecar = true }, preview_controller: "SomeOtherController", @@ -727,11 +731,11 @@ def test_config_options_shared_between_base_and_engine show_previews_source: true }.each do |option, value| with_config_option(option, value, config_entrypoint: config) do - assert_equal(config.public_send(option), config_entrypoints.second.public_send(option)) + assert_equal(config.public_send(option), second_entrypoint.public_send(option), + "#{first_entrypoint_name} should be the same as #{second_entrypoint_name} for #{option.inspect}") end end end - config_entrypoints.rotate! end end diff --git a/test/sandbox/test/rendering_test.rb b/test/sandbox/test/rendering_test.rb index 6221b68da..573403c87 100644 --- a/test/sandbox/test/rendering_test.rb +++ b/test/sandbox/test/rendering_test.rb @@ -16,8 +16,8 @@ def test_render_inline_allocations MyComponent.ensure_compiled allocations = (Rails.version.to_f >= 8.0) ? - {"3.5.0" => 117, "3.4.1" => 117, "3.3.7" => 129} : - {"3.3.7" => 120, "3.3.0" => 120, "3.2.6" => 118, "3.1.6" => 118, "3.0.7" => 127} + {"3.5.0" => 117, "3.4.1" => 117, "3.3.7" => 128} : + {"3.3.7" => 119, "3.3.0" => 119, "3.2.6" => 117, "3.1.6" => 117, "3.0.7" => 126} assert_allocations(**allocations) do render_inline(MyComponent.new) diff --git a/test/test_engine/test_helper.rb b/test/test_engine/test_helper.rb index 4dde4ed40..fb5806d89 100644 --- a/test/test_engine/test_helper.rb +++ b/test/test_engine/test_helper.rb @@ -22,7 +22,7 @@ Rails::Generators.namespace = TestEngine -def with_config_option(option_name, new_value, config_entrypoint: TestEngine::Engine.config.view_component) +def with_config_option(option_name, new_value, config_entrypoint: ViewComponent::GlobalConfig) old_value = config_entrypoint.public_send(option_name) config_entrypoint.public_send(:"#{option_name}=", new_value) yield