From 941da31c5d0ba274380a111b736beada05a4ea1f Mon Sep 17 00:00:00 2001 From: Henning Koch Date: Fri, 14 Jun 2024 12:54:12 +0200 Subject: [PATCH 1/5] Default preview controller should not crash with `config.action_controller.include_all_helpers = false` (fixes #1654) --- app/controllers/concerns/view_component/preview_actions.rb | 7 ++++++- docs/CHANGELOG.md | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/view_component/preview_actions.rb b/app/controllers/concerns/view_component/preview_actions.rb index f27d175e6..407e05b7d 100644 --- a/app/controllers/concerns/view_component/preview_actions.rb +++ b/app/controllers/concerns/view_component/preview_actions.rb @@ -14,7 +14,12 @@ module PreviewActions # Including helpers here ensures that we're loading the # latest version of helpers if code-reloading is enabled - helper :all if include_all_helpers + if include_all_helpers + helper :all + else + # Always provide the #view_source helper + helper PreviewHelper + end end def index diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index c32e80083..e7d604573 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -26,6 +26,11 @@ nav_order: 5 *Reegan Viljoen* +* Fix a bug where component previews would crash with "undefined local variable or method `preview_source'" + + *Henning Koch* + + ## 3.12.1 * Ensure content is rendered correctly for forwarded slots. From c0faca63f37580901348687eda177782076ff522 Mon Sep 17 00:00:00 2001 From: Henning Koch Date: Fri, 14 Jun 2024 12:57:55 +0200 Subject: [PATCH 2/5] PreviewHelper: Rename private #find_template_data method to something less generic The default Rails configuration loads the PreviewHelper for every controller. We should use specific method names to prevent name clashes with user-provided helpers. --- app/helpers/preview_helper.rb | 2 +- app/views/view_components/_preview_source.html.erb | 2 +- test/sandbox/test/preview_helper_test.rb | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/helpers/preview_helper.rb b/app/helpers/preview_helper.rb index 18016ff76..bac7557bc 100644 --- a/app/helpers/preview_helper.rb +++ b/app/helpers/preview_helper.rb @@ -22,7 +22,7 @@ def prism_js_source_url serve_static_preview_assets? ? asset_path("prism.min.js", skip_pipeline: true) : "https://cdn.jsdelivr.net/npm/prismjs@1.28.0/prism.min.js" end - def find_template_data(lookup_context:, template_identifier:) + def find_template_data_for_preview_source(lookup_context:, template_identifier:) template = lookup_context.find_template(template_identifier) if Rails.version.to_f >= 6.1 || template.source.present? diff --git a/app/views/view_components/_preview_source.html.erb b/app/views/view_components/_preview_source.html.erb index 606b8ac80..65675189a 100644 --- a/app/views/view_components/_preview_source.html.erb +++ b/app/views/view_components/_preview_source.html.erb @@ -7,7 +7,7 @@ <%= h @preview.preview_source(@example_name) %> <% else %> - <% template_data = find_template_data(lookup_context: @view_renderer.lookup_context, template_identifier: @render_args[:template]) %> + <% template_data = find_template_data_for_preview_source(lookup_context: @view_renderer.lookup_context, template_identifier: @render_args[:template]) %> <%= h template_data[:source] %> diff --git a/test/sandbox/test/preview_helper_test.rb b/test/sandbox/test/preview_helper_test.rb index 5921a525d..8ba5087ba 100644 --- a/test/sandbox/test/preview_helper_test.rb +++ b/test/sandbox/test/preview_helper_test.rb @@ -17,7 +17,7 @@ def test_returns_template_data_with_no_template lookup_context = Minitest::Mock.new lookup_context.expect(:find_template, mock_template, [template_identifier]) - template_data = PreviewHelper.find_template_data( + template_data = PreviewHelper.find_template_data_for_preview_source( lookup_context: lookup_context, template_identifier: template_identifier ) @@ -40,7 +40,7 @@ def test_returns_template_data_with_template_of_different_languages lookup_context = Minitest::Mock.new lookup_context.expect(:find_template, mock_template, [template_identifier]) - template_data = PreviewHelper.find_template_data( + template_data = PreviewHelper.find_template_data_for_preview_source( lookup_context: lookup_context, template_identifier: template_identifier ) @@ -68,7 +68,7 @@ def test_returns_template_data_without_dedicated_template mock = Minitest::Mock.new mock.expect :map, [expected_template_path] ViewComponent::Base.stub(:preview_paths, mock) do - template_data = PreviewHelper.find_template_data( + template_data = PreviewHelper.find_template_data_for_preview_source( lookup_context: lookup_context, template_identifier: template_identifier ) @@ -96,7 +96,7 @@ def test_returns_template_data_with_dedicated_template mock.expect :map, [expected_template_path] Rails.application.config.view_component.stub(:preview_paths, mock) do File.stub(:read, expected_source, [expected_template_path]) do - template_data = PreviewHelper.find_template_data( + template_data = PreviewHelper.find_template_data_for_preview_source( lookup_context: lookup_context, template_identifier: template_identifier ) @@ -122,7 +122,7 @@ def test_raises_with_no_matching_template mock.expect :map, [] Rails.application.config.view_component.stub :preview_paths, mock do exception = assert_raises ViewComponent::NoMatchingTemplatesForPreviewError do - PreviewHelper.find_template_data( + PreviewHelper.find_template_data_for_preview_source( lookup_context: lookup_context, template_identifier: template_identifier ) @@ -146,7 +146,7 @@ def test_raises_with_conflict_in_template_resolution mock.expect :map, [template_identifier + ".html.haml", template_identifier + ".html.erb"] Rails.application.config.view_component.stub :preview_paths, mock do exception = assert_raises ViewComponent::MultipleMatchingTemplatesForPreviewError do - PreviewHelper.find_template_data( + PreviewHelper.find_template_data_for_preview_source( lookup_context: lookup_context, template_identifier: template_identifier ) From 40574af2534c6afc8bb8aa64e32cccfb6cd4c55c Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 10 Jan 2025 16:18:49 -0700 Subject: [PATCH 3/5] Apply suggestions from code review --- docs/CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index e40461327..7e33b09a6 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -226,11 +226,10 @@ nav_order: 5 *Reegan Viljoen* -* Fix a bug where component previews would crash with "undefined local variable or method `preview_source'" +* Fix a bug where component previews would crash with "undefined local variable or method `preview_source`". *Henning Koch* - ## 3.12.1 * Ensure content is rendered correctly for forwarded slots. From 6aeee27bad30b27809663f318ddf9092b6a1d41b Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 10 Jan 2025 16:22:59 -0700 Subject: [PATCH 4/5] add nocov --- app/controllers/concerns/view_component/preview_actions.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/concerns/view_component/preview_actions.rb b/app/controllers/concerns/view_component/preview_actions.rb index 407e05b7d..1a4e7536e 100644 --- a/app/controllers/concerns/view_component/preview_actions.rb +++ b/app/controllers/concerns/view_component/preview_actions.rb @@ -17,8 +17,10 @@ module PreviewActions if include_all_helpers helper :all else + # :nocov: # Always provide the #view_source helper helper PreviewHelper + # :nocov: end end From e7788a7c5d52358fdbed75a1eb7644f9a92657a9 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 10 Jan 2025 16:25:15 -0700 Subject: [PATCH 5/5] Update docs/CHANGELOG.md --- docs/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 7e33b09a6..25475738c 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -226,7 +226,7 @@ nav_order: 5 *Reegan Viljoen* -* Fix a bug where component previews would crash with "undefined local variable or method `preview_source`". +* Fix a bug where component previews would crash with "undefined local variable or method `preview_source`." *Henning Koch*