Merge pull request #130 from dry-rb/refactor/clarify-template-lookup

Clarify config handling & template lookup

lib/dry/view.rb

@@ -7,6 +7,7 @@
 
 require_relative "view/context"
 require_relative "view/exposures"
+require_relative "view/errors"
 require_relative "view/part_builder"
 require_relative "view/path"
 require_relative "view/render_environment"
@@ -33,9 +34,6 @@ module Dry
   #
   # @api public
   class View
-    # @api private
-    UndefinedTemplateError = Class.new(StandardError)
-
     # @api private
     DEFAULT_RENDERER_OPTIONS = {default_encoding: "utf-8"}.freeze
 
@@ -459,7 +457,7 @@ def config
     # @return [Rendered] rendered view object
     # @api public
     def call(format: config.default_format, context: config.default_context, **input)
-      raise UndefinedTemplateError, "no +template+ configured" unless config.template
+      ensure_config
 
       env = self.class.render_env(format: format, context: context)
       template_env = self.class.template_env(format: format, context: context)
@@ -469,14 +467,20 @@ def call(format: config.default_format, context: config.default_context, **input
 
       if layout?
         layout_env = self.class.layout_env(format: format, context: context)
-        output = layout_env.template(self.class.layout_path, layout_env.scope(config.scope, layout_locals(locals))) { output }
+        output = env.template(self.class.layout_path, layout_env.scope(config.scope, layout_locals(locals))) { output }
       end
 
       Rendered.new(output: output, locals: locals)
     end
 
     private
 
+    # @api private
+    def ensure_config
+      raise UndefinedConfigError.new(:paths) unless Array(config.paths).any?
+      raise UndefinedConfigError.new(:template) unless config.template
+    end
+
     # @api private
     def locals(render_env, input)
       exposures.(context: render_env.context, **input) do |value, exposure|

lib/dry/view/errors.rb

@@ -0,0 +1,27 @@
+module Dry
+  class View
+    # Error raised when critical settings are not configured
+    #
+    # @api private
+    class UndefinedConfigError < StandardError
+      def initialize(key)
+        super("no +#{key}+ configured")
+      end
+    end
+
+    # Error raised when template could not be found within a view's configured
+    # paths
+    #
+    # @api private
+    class TemplateNotFoundError < StandardError
+      def initialize(template_name, lookup_paths)
+        msg = [
+          "Template +#{template_name}+ could not be found in paths:",
+          lookup_paths.map { |path| " - #{path}"}
+        ].join("\n\n")
+
+        super(msg)
+      end
+    end
+  end
+end

lib/dry/view/path.rb

@@ -25,11 +25,11 @@ def initialize(dir, root: dir)
         @root = Pathname(root)
       end
 
-      def lookup(name, format, include_shared: true)
-        fetch_or_store(dir, root, name, format) do
-          template?(name, format) ||
-            (include_shared && template?("shared/#{name}", format)) ||
-            !root? && chdir("..").lookup(name, format)
+      def lookup(name, format, child_dirs: [], parent_dir: false)
+        fetch_or_store(dir, root, name, format, child_dirs, parent_dir) do
+          lookup_template(name, format) ||
+            lookup_in_child_dirs(name, format, child_dirs: child_dirs) ||
+            parent_dir && lookup_in_parent_dir(name, format, child_dirs: child_dirs)
         end
       end
 
@@ -48,10 +48,21 @@ def root?
       end
 
       # Search for a template using a wildcard for the engine extension
-      def template?(name, format)
+      def lookup_template(name, format)
         glob = dir.join("#{name}.#{format}.*")
         Dir[glob].first
       end
+
+      def lookup_in_child_dirs(name, format, child_dirs:)
+        child_dirs.reduce(nil) { |_, dir|
+          template = chdir(dir).lookup(name, format)
+          break template if template
+        }
+      end
+
+      def lookup_in_parent_dir(name, format, child_dirs:)
+        !root? && chdir("..").lookup(name, format, child_dirs: child_dirs, parent_dir: true)
+      end
     end
   end
 end

lib/dry/view/renderer.rb

@@ -2,6 +2,7 @@
 
 require "dry/core/cache"
 require "dry/equalizer"
+require_relative "errors"
 require_relative "tilt"
 
 module Dry
@@ -15,8 +16,6 @@ class Renderer
 
       include Dry::Equalizer(:paths, :format, :engine_mapping, :options)
 
-      TemplateNotFoundError = Class.new(StandardError)
-
       attr_reader :paths, :format, :engine_mapping, :options
 
       def initialize(paths, format:, engine_mapping: nil, **options)
@@ -26,19 +25,24 @@ def initialize(paths, format:, engine_mapping: nil, **options)
         @options = options
       end
 
-      def template(name, scope, &block)
-        path = lookup(name)
+      def template(name, scope, **lookup_options, &block)
+        path = lookup(name, **lookup_options)
 
         if path
           render(path, scope, &block)
         else
-          msg = "Template #{name.inspect} could not be found in paths:\n#{paths.map { |pa| "- #{pa.to_s}" }.join("\n")}"
-          raise TemplateNotFoundError, msg
+          raise TemplateNotFoundError.new(name, paths)
         end
       end
 
       def partial(name, scope, &block)
-        template(name_for_partial(name), scope, &block)
+        template(
+          name_for_partial(name),
+          scope,
+          child_dirs: %w[shared],
+          parent_dir: true,
+          &block
+        )
       end
 
       def render(path, scope, &block)
@@ -51,15 +55,15 @@ def chdir(dirname)
         self.class.new(new_paths, format: format, **options)
       end
 
-      def lookup(name)
-        paths.inject(false) { |_, path|
-          result = path.lookup(name, format, include_shared: false)
+      private
+
+      def lookup(name, **options)
+        paths.inject(nil) { |_, path|
+          result = path.lookup(name, format, **options)
           break result if result
         }
       end
 
-      private
-
       def name_for_partial(name)
         name_segments = name.to_s.split(PATH_DELIMITER)
         name_segments[0..-2].push("#{PARTIAL_PREFIX}#{name_segments[-1]}").join(PATH_DELIMITER)

spec/fixtures/integration/errors/hello.html.slim

@@ -0,0 +1 @@
+h1 Hello

spec/integration/view/errors_spec.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+require "dry/view"
+require "dry/view/context"
+
+RSpec.describe "View / errors" do
+  specify "Raising an error when paths are not configured" do
+    view = Class.new(Dry::View) do
+      config.template = "hello"
+    end.new
+
+    expect { view.() }.to raise_error(Dry::View::UndefinedConfigError, "no +paths+ configured")
+  end
+
+  specify "Raising an error when template is not configured" do
+    view = Class.new(Dry::View) do
+      config.paths = FIXTURES_PATH.join("integration/errors")
+    end.new
+
+    expect { view.() }.to raise_error(Dry::View::UndefinedConfigError, "no +template+ configured")
+  end
+end

spec/unit/renderer_spec.rb

@@ -22,17 +22,13 @@
     it 'does not include `shared/` subdirectory under root when looking up templates' do
       expect {
         renderer.template(:_shared_hello, scope)
-      }.to raise_error(Dry::View::Renderer::TemplateNotFoundError, /_shared_hello/)
-    end
-
-    it 'renders template in shared/ when descending from an upper directory' do
-      expect(renderer.chdir('nested').template(:_shared_hello, scope)).to eql('<h1>Hello</h1>')
+      }.to raise_error(Dry::View::TemplateNotFoundError, /_shared_hello/)
     end
 
     it 'raises error when template cannot be found' do
       expect {
         renderer.template(:missing_template, scope)
-      }.to raise_error(Dry::View::Renderer::TemplateNotFoundError, /missing_template/)
+      }.to raise_error(Dry::View::TemplateNotFoundError, /missing_template/)
     end
   end
 
@@ -41,10 +37,8 @@
       expect(renderer.partial(:hello, scope)).to eql('<h1>Partial hello</h1>')
     end
 
-    it 'does not include `shared/` subdirectory under root when looking up partials' do
-      expect {
-        renderer.partial(:shared_hello, scope)
-      }.to raise_error(Dry::View::Renderer::TemplateNotFoundError, /_shared_hello/)
+    it 'renders partial in shared/ subdirectory under root' do
+      expect(renderer.chdir('hello').partial(:shared_hello, scope)).to eql('<h1>Hello</h1>')
     end
 
     it 'renders partial in shared/ subdirectory when descending from an upper directory' do
@@ -62,7 +56,7 @@
     it 'raises error when partial cannot be found' do
       expect {
         renderer.partial(:missing_partial, scope)
-      }.to raise_error(Dry::View::Renderer::TemplateNotFoundError, /_missing_partial/)
+      }.to raise_error(Dry::View::TemplateNotFoundError, /_missing_partial/)
     end
   end
 

spec/unit/view_spec.rb

@@ -33,14 +33,6 @@ def title
         '<!DOCTYPE html><html><head><title>Test</title></head><body><h1>User</h1><p>Jane</p></body></html>'
       )
     end
-
-    it 'provides a meaningful error if the template name is missing' do
-      view = Class.new(Dry::View) do
-        config.paths = SPEC_ROOT.join('fixtures/templates')
-      end.new
-
-      expect { view.(context: context) }.to raise_error Dry::View::UndefinedTemplateError
-    end
   end
 
   describe 'renderer options' do