Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Limit templates to defined file extension #1990

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/guide/templates.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,15 @@ class MyComponent < ViewComponent::Base
strip_trailing_whitespace(false)
end
```

## Setting template file extension

Sometimes a component will warrant the use of multiple file types for a single template handler. An example would be ERB handled files for both HTML and CSS. By default ViewComponent will return a "More than one template found for" error. To avoid this set the `VC_TEMPLATE_EXTENSION` constant in your component to the file extension of the template you wish your component to use.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we wouldn't want to change the existing logic to look for multiple extensions and unique by that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BlakeWilliams I'm not clear on why a component would need multiple template file extensions. Could you provide an example to help me understand?

My understanding and experience is that a component renders a single type of output, usually HTML. So the variant rules apply to that HTML file type/extension if varying outputs are needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why a component would care if there's both an HTML and a CSS file associated with it. The only case I think we care about is having two of the same filetype, like foo.html.erb and foo.html.haml, for example.

We also already have a way to determine the type of content a component should be rendering, via format

def format
:html
end
(at the class level)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BlakeWilliams We're on the same page. The component should only care about what you mentioned. However the current functionality starts with the handler/processor and not the filetype such as .html.*.

Hence why there's a need to add a bit of code to limit the scope. This is to avoid the errors that arise if a component has more than one filetype per handler/processor.

Regarding the format at the class level you pointed out I did see that but I'm not certain it does much currently. However it doesn't mean that it shouldn't be leveraged in the solution we're aiming for. So tweaking things to use it is likely a good idea.


```ruby
class MyComponent < ViewComponent::Base
VC_TEMPLATE_EXTENSION = 'html'

...
end
```
26 changes: 23 additions & 3 deletions lib/view_component/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,19 @@ class << self
# @private
attr_accessor :source_location, :virtual_path

# If the component class sets the template extension, use it
#
# A component overrides the global default by setting the
# VC_TEMPLATE_EXTENSION constant of the component class
#
def template_ext
if const_defined?(:VC_TEMPLATE_EXTENSION)
return self::VC_TEMPLATE_EXTENSION
end

template_extension
end

# Find sidecar files for the given extensions.
#
# The provided array of extensions is expected to contain
Expand All @@ -463,6 +476,13 @@ def sidecar_files(extensions)
filename = File.basename(source_location, ".rb")
component_name = name.demodulize.underscore

# Limit template type if set in component configuration
component_template_extension_glob = ""

unless template_ext.nil?
component_template_extension_glob = ".*{#{template_ext}}"
end

# Add support for nested components defined in the same file.
#
# for example
Expand All @@ -475,15 +495,15 @@ def sidecar_files(extensions)
# Without this, `MyOtherComponent` will not look for `my_component/my_other_component.html.erb`
nested_component_files =
if name.include?("::") && component_name != filename
Dir["#{directory}/#{filename}/#{component_name}.*{#{extensions}}"]
Dir["#{directory}/#{filename}/#{component_name}#{component_template_extension_glob}.*{#{extensions}}"]
else
[]
end

# view files in the same directory as the component
sidecar_files = Dir["#{directory}/#{component_name}.*{#{extensions}}"]
sidecar_files = Dir["#{directory}/#{component_name}#{component_template_extension_glob}.*{#{extensions}}"]

sidecar_directory_files = Dir["#{directory}/#{component_name}/#{filename}.*{#{extensions}}"]
sidecar_directory_files = Dir["#{directory}/#{component_name}/#{filename}#{component_template_extension_glob}.*{#{extensions}}"]

(sidecar_files - [source_location] + sidecar_directory_files + nested_component_files).uniq
end
Expand Down
8 changes: 7 additions & 1 deletion lib/view_component/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ def defaults
preview_paths: default_preview_paths,
test_controller: "ApplicationController",
default_preview_layout: nil,
capture_compatibility_patch_enabled: false
capture_compatibility_patch_enabled: false,
template_extension: nil
})
end

Expand Down Expand Up @@ -167,6 +168,11 @@ def defaults
# previews.
# Defaults to `false`.

# @!attribute template_extension
# @return [String]
# Define which file extension is to be used as the template.
# Defaults to `nil`.

def default_preview_paths
return [] unless defined?(Rails.root) && Dir.exist?("#{Rails.root}/test/components/previews")

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.custom-selector {
background-image: url(<%=asset_path"background_test.png"%>);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<div>
<%= content %>
<%= @message %>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

class TemplateExtensionDefinedComponent < ViewComponent::Base
VC_TEMPLATE_EXTENSION = "html"

def initialize(message:)
@message = message
end
end
7 changes: 7 additions & 0 deletions test/sandbox/test/rendering_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1116,6 +1116,13 @@ def test_use_helper
assert_selector ".helper__message", text: "Hello helper method"
end

def test_renders_when_more_than_one_handler_file_exists_but_template_extension_has_been_defined
render_inline(TemplateExtensionDefinedComponent.new(message: "bar")) { "foo" }

assert_text("foo")
assert_text("bar")
end

def test_inline_component_renders_without_trailing_whitespace
without_template_annotations do
render_inline(InlineTrailingWhitespaceComponent.new)
Expand Down
Loading