Ensure html format when rendering combobox partial

[szareey]

This PR fixes an issue where HotwireCombobox fails to render correctly when a form partial is rendered within a turbo_stream response. Because the response format is turbo_stream, lookup_context.formats is set to :turbo_stream and the gem attempts to locate a _component.turbo_stream.erb partial (instead of _components.html.erb), resulting in a missing template error.

This PR introduces a wrapper method that forces render calls within the gem to use HTML format regardless of the outer response format, and then restores lookup_context.formats to its original value after rendering. If additional render calls are added in the future, they should also utilize this wrapper.

[josefarias]

Hey @szareey thanks for the PR!

So in the past I've refrained from introducing something like this because it obfuscates what's actually happening.

The response format must be a turbo-stream because otherwise Turbo won't know how to interpret it. Ideally the user should be very aware of this.

Adding html to the lookup context invites confusion for more inexperienced users, prompting questions like "why does this work for the combobox, but not my app?" (It's because the combobox does some magic behind the scenes!). Whereas the current behavior might be frustrating, at least it uncovers a gap in the user's mental model.

I'd rather stick to convention here and rely on improving the docs to make this more obvious. So closing for now. But I'm open to hearing a case for why we should do otherwise.

[szareey]

Hi @josefarias, thanks for the prompt follow up.

Your point about obfuscation makes sense, and I agree it's best to keep things explicit. That said, I'd love some clarification. If a user has an edit action that returns a turbo_stream response, and as part of that turbo stream response a form is rendered that uses HotwireCombobox, how else can we prevent HotwireCombobox's render call from attempting to load a HotwireCombobox _component.turbo_stream.erb? Currently the behaviour results in a missing template error.

I see two possible solutions:

1. Adding a '_components.turbo_stream.erb' template
2. Modifying lookup_context to ensure the combobox always renders as HTML

However, if there's another preferred way to handle this I'd love to learn about it.

[josefarias]

Ah I see what you mean. I think I misunderstood the initial motivator for this PR. This is indeed a bug.

In this case I think we can do this instead:

diff --git a/lib/hotwire_combobox/helper.rb b/lib/hotwire_combobox/helper.rb
index 5f7ce04..818aa13 100644
--- a/lib/hotwire_combobox/helper.rb
+++ b/lib/hotwire_combobox/helper.rb
@@ -17,7 +17,7 @@ module HotwireCombobox
     def hw_combobox_tag(name, options_or_src = [], render_in: {}, include_blank: nil, **kwargs, &block)
       options, src = hw_extract_options_and_src options_or_src, render_in, include_blank
       component = HotwireCombobox::Component.new self, name, options: options, async_src: src, request: request, **kwargs
-      render component, &block
+      render component, formats: [:html], &block
     end

Would you like to submit a separate PR with that fix + a quick system test, @szareey? No worries if not, just giving you first dibs since you found the issue

[szareey]

Thanks for that, I appreciate the dibs!

As for your suggested solution, that was exactly what I first tried, but it didn't work. My best guess is that if we were directly rendering a partial like render "hotwire_combobox/component", formats: [:html] then that's a context where formats: [:html] would work as expected. However, since we're rendering an object instead (render component), Rails determines the partial to render based on the lookup_context.formats which is still set to :turbo_stream from the surrounding request.

If you really don't like the wrapper idea, I understand. There is another solution to create a component.turbo_stream.erb which simply renders _component.html.erb, I haven't tried that solution yet though.

[josefarias]

Ah interesting, I didn't realize that was the case when rendering objects. I'll take a closer look and circle back!

[josefarias]

@szareey sorry about the delay, I’m back now. I want to make sure we address the root cause of your problem. Could you add a failing test to this PR so I can take a closer look please? A reproduction repo would also work.

[josefarias]

Thanks a bunch @szareey taking a look tonight

[szareey]

Hi Jose,

Thanks for the follow up. In creating the reproduction I realized that my issue is more niche than I thought.

In my app, I'm wrapping my turbo_stream calls to implement animation/transitions when rendering from a turbo_stream. The combobox partial error only appears if I've wrapped a combobox component in this. However, if I render a combobox directly from a standard turbo_stream method there is no error. I've reproduced both scenarios in the gem's dummy app under turbo_stream_rendering

So boss, I'm not sure if my issue is general enough to warrant a fix, unless my issue points to a bigger issue. Or whether the fix is benign enough to implement though the issue may be niche.

Sana

[szareey]

This works

[szareey]

This does not work

[josefarias]

Thanks @szareey so it looks like Rails can't find the partial when the component is rendered inside a captured block in a turbo_stream response. You’re right, it’s fairly specific! But a bug nonetheless — and with an easy fix at that.

I’ve pushed up some changes simplifying the test and implementing a fix.

re: the difference in rendering objects vs partials, and how Rails was ignoring the formats kwarg — I forgot Rails simply calls #render_in on the object, letting the object define all the rendering options itself. So it was just a matter of moving the kwarg to #render_in.

[josefarias]

Unused, removed in main