Default preview controller should not crash with `config.action_contr…

…oller.include_all_helpers = false` (#2045) * Default preview controller should not crash with `config.action_controller.include_all_helpers = false` (fixes #1654) * 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. * Apply suggestions from code review * add nocov * Update docs/CHANGELOG.md --------- Co-authored-by: Joel Hawksley <[email protected]> Co-authored-by: Joel Hawksley <[email protected]>
ViewComponent · Jan 10, 2025 · eba4c14 · eba4c14
1 parent c057d4b
commit eba4c14
Show file tree

Hide file tree

Showing 5 changed files with 20 additions and 9 deletions.
diff --git a/app/controllers/concerns/view_component/preview_actions.rb b/app/controllers/concerns/view_component/preview_actions.rb
@@ -14,7 +14,14 @@ 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
+        # :nocov:
+        # Always provide the #view_source helper
+        helper PreviewHelper
+        # :nocov:
+      end
     end
 
     def index

diff --git 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/[email protected]/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
@@ -7,7 +7,7 @@
         <%= h @preview.preview_source(@example_name) %>
       </code>
     <% 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]) %>
       <code class="language-<%= template_data[:prism_language_name] %>">
         <%= h template_data[:source] %>
       </code>

diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -226,6 +226,10 @@ 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.

diff --git 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
           )