From 5d2bce286b81441bebf838e072e190e9a8d60ebf Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Oct 2026 11:42:05 -0600 Subject: [PATCH] Validate configuration Hash mutation methods --- docs/CHANGELOG.md | 2 +- docs/guide/generators.md | 2 +- lib/view_component/config.rb | 42 +++++++++- test/sandbox/test/config_test.rb | 129 +++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+), 5 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index ab2484fdb..f252f9a60 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -14,7 +14,7 @@ nav_order: 6 *Joel Hawksley* -* Raise `ViewComponent::UnknownConfigOptionError` when setting a `config.view_component` option that doesn't exist, rather than ignoring it. This includes options renamed or removed in 4.0, such as `preview_controller` (now `previews.controller`), and the error names the replacement. Replacing `previews` or `generate` with a hash preserves this validation. +* Raise `ViewComponent::UnknownConfigOptionError` when setting a `config.view_component` option that doesn't exist, rather than ignoring it. This includes options renamed or removed in 4.0, such as `preview_controller` (now `previews.controller`), and the error names the replacement. Replacing `previews` or `generate` with a hash preserves this validation, including through Hash mutation methods such as `merge!`, `update`, `store`, and `replace`. *Fiona Burcombe* diff --git a/docs/guide/generators.md b/docs/guide/generators.md index 965e16bed..1ead331fb 100644 --- a/docs/guide/generators.md +++ b/docs/guide/generators.md @@ -192,7 +192,7 @@ bin/rails generate view_component:component Example title content --parent MyBas create app/components/example_component.html.erb ``` -To always use a specific parent class, set `config.view_component.parent_class = "MyBaseComponent"`. +To always use a specific parent class, set `config.view_component.generate.parent_class = "MyBaseComponent"`. ### Skip collision check diff --git a/lib/view_component/config.rb b/lib/view_component/config.rb index 16632cf18..09d82c6b7 100644 --- a/lib/view_component/config.rb +++ b/lib/view_component/config.rb @@ -15,19 +15,22 @@ def initialize(namespace, allowed_keys, default: nil, renamed: {}, removed: {}) @allowed_keys = allowed_keys @renamed = renamed @removed = removed + @nested_options = {} end def []=(key, value) key = key.to_sym if @allowed_keys.include?(key) - if self[key].is_a?(Options) + @nested_options[key] ||= value if value.is_a?(Options) + if (nested_options = @nested_options[key]) raise TypeError, "#{@namespace}.#{key} must be a Hash" unless value.is_a?(Hash) - value = self[key].dup.clear.tap do |options| + value = nested_options.dup.clear.tap do |options| value.each_pair { |nested_key, nested_value| options[nested_key] = nested_value } end end + super else raise UnknownConfigOptionError.new( @@ -39,6 +42,39 @@ def []=(key, value) ) end end + + alias_method :store, :[]= + + def merge!(*others) + others.each do |other| + {}.merge!(other).each_pair do |key, value| + key = key.to_sym + value = yield(key, self[key], value) if block_given? && key?(key) + self[key] = value + end + end + self + end + + alias_method :update, :merge! + + def replace(other) + replacement = dup.clear + replacement.merge!(other) + super(replacement) + end + + def transform_keys!(*args, &block) + return enum_for(:transform_keys!) if args.empty? && !block + + replace(to_h.transform_keys!(*args, &block)) + end + + def transform_values!(&block) + return enum_for(:transform_values!) unless block + + replace(to_h.transform_values!(&block)) + end end # Options renamed in 4.0, mapped to their new name. @@ -222,7 +258,7 @@ def default_generate_options "#{Options::ROOT_NAMESPACE}.generate", %i[path preview_path parent_class sidecar stimulus_controller typescript locale distinct_locale_files preview use_component_path_for_rspec_tests], default: false, - renamed: RENAMED_OPTIONS + renamed: {component_parent_class: RENAMED_OPTIONS.fetch(:component_parent_class)} ) options.preview_path = "" options.path = "app/components" diff --git a/test/sandbox/test/config_test.rb b/test/sandbox/test/config_test.rb index 2cfbd8903..fb7ed77a6 100644 --- a/test/sandbox/test/config_test.rb +++ b/test/sandbox/test/config_test.rb @@ -69,6 +69,135 @@ def test_setting_an_option_removed_in_4_0_raises ) end + def test_generate_options_do_not_use_root_rename_hints + error = assert_raises(ViewComponent::UnknownConfigOptionError) do + @config.generate.preview_paths = "spec/components/previews" + end + + assert_includes error.message, "`config.view_component.generate.preview_paths` isn't a ViewComponent configuration option." + assert_includes error.message, "Valid options are: path, preview_path, parent_class" + refute_includes error.message, "was renamed" + end + + def test_hash_writers_validate_keys + %i[merge! update replace].each do |method| + options = ViewComponent::Config.defaults + assert_raises(ViewComponent::UnknownConfigOptionError) do + options.public_send(method, preview_controller: "MyPreviewController") + end + end + + assert_raises(ViewComponent::UnknownConfigOptionError) do + @config.store(:test_controller, "MyController") + end + end + + def test_hash_writers_wrap_nested_namespaces + %i[merge! update replace store].each do |method| + options = ViewComponent::Config.defaults + if method == :store + options.store("generate", {"parent_class" => "MyBaseComponent"}) + else + assert_same options, options.public_send(method, "generate" => {"parent_class" => "MyBaseComponent"}) + end + + assert_equal "MyBaseComponent", options.generate.parent_class + assert_equal false, options.generate.typescript + assert_raises(ViewComponent::UnknownConfigOptionError) { options.generate.typo = true } + assert_raises(ViewComponent::UnknownConfigOptionError) do + if method == :store + options.generate.store(:typo, true) + else + options.generate.public_send(method, typo: true) + end + end + end + end + + def test_hash_writers_reject_invalid_nested_values + %i[merge! update replace store].each do |method| + options = ViewComponent::Config.defaults + [nil, {contoller: "MyPreviewController"}].each do |value| + original = options.previews + error = value.nil? ? TypeError : ViewComponent::UnknownConfigOptionError + assert_raises(error) do + if method == :store + options.store(:previews, value) + else + options.public_send(method, previews: value) + end + end + assert_same original, options.previews + end + end + end + + def test_merge_accepts_multiple_hashes_and_a_conflict_block + options = @config.generate + conflicts = [] + result = options.update({"path" => "lib/components"}, {path: "other/components", sidecar: true}) do |key, old, new| + conflicts << [key, old, new] + "#{old}/#{new}" + end + + assert_same options, result + assert_equal "app/components/lib/components/other/components", options.path + assert_equal true, options.sidecar + assert_equal [ + [:path, "app/components", "lib/components"], + [:path, "app/components/lib/components", "other/components"] + ], conflicts + assert_same options, options.merge! + assert_raises(TypeError) { options.merge!(nil) } + assert_raises(TypeError) { options.replace(nil) } + end + + def test_replacing_options_validates_before_removing_existing_values + original = @config.previews.dup + + assert_raises(ViewComponent::UnknownConfigOptionError) do + @config.previews.replace(controller: "MyPreviewController", typo: true) + end + + assert_equal original, @config.previews + end + + def test_removed_namespaces_retain_their_validation_when_reassigned + %i[clear replace delete shift].each do |method| + options = ViewComponent::Config.defaults + case method + when :replace then options.replace({}) + when :delete then options.delete(:generate) + else options.public_send(method) + end + replacement = {generate: {parent_class: "MyBaseComponent"}} + options.merge!(replacement) + + assert_equal "MyBaseComponent", options.generate.parent_class + assert_equal false, options.generate.typescript + assert_raises(ViewComponent::UnknownConfigOptionError) { options.generate.typo = true } + end + end + + def test_transforming_keys_validates_new_keys + options = @config.generate + assert_kind_of Enumerator, options.transform_keys! + assert_same options, options.transform_keys!(&:to_s) + assert_raises(ViewComponent::UnknownConfigOptionError) do + options.transform_keys!(path: :typo) + end + assert_equal "app/components", options.path + end + + def test_transforming_values_preserves_nested_validation + options = ViewComponent::Config.defaults + assert_kind_of Enumerator, options.transform_values! + assert_same options, options.transform_values! { |value| value.is_a?(Hash) ? value.to_h : value } + assert_equal false, options.generate.typescript + assert_raises(ViewComponent::UnknownConfigOptionError) { options.previews.typo = true } + assert_raises(TypeError) { options.transform_values! { nil } } + end + def test_setting_a_misspelled_option_raises error = assert_raises(ViewComponent::UnknownConfigOptionError) do @config.instrumentaton_enabled = true