add default_format method to RailsTemplate for Turbo compatibility

[lazylester]

as discussed in 'issues'

[k0kubun]

Thanks!

[timdiggins]

I think this change should be rolled back for the following reasons:

• Not all uses of Haml are to create html content-type responses and this can cause issues for them (e.g. js or js with inline html).

• It seems like dhh's comments on the original issue in turbo-rails just before he closed it (Wrong Content-Type with HAML hotwired/turbo-rails#287 (comment)) were to say that the naming convention (including .html. in view template names) is required.

• I don't think Haml should have code that is there to just make Turbo-rails work for people who don't want to follow this convention. And it's pretty easy to rename all your xxx.haml files to xxx.html.haml files - or to add a monkey patch in your app.

NB: "default_format" is quite underdocumented and seems confusingly named - when you define it you prevent the actual default fallback format (which is empty) from being used ( see ActionView::TemplateDetails::Requested.format_or_default)

[lazylester]

@timdiggins haml has historically always supported #default_format. If it's considered appropriate to remove this support, as happened when hamlit was merged, then it should first be deprecated, so as to smooth the path for developers who are upgrading.

If restoring support for '#content_type` is causing problems in some scenarios (I'm not at all clear on the js scenario you mention), then it must always have been problematic, surely?

Whilst DHH may assert that inclusion of .html in view templates is required, it actually never has been a requirement, but simply common practice.

[k0kubun]

Can either of you work on a pull request? As long as it passes tests, I'm open to merging one.

[timdiggins]

Long response - hope it's helpful.

@k0kubun I'm not sure what the pull request would be - I'm proposing reverting this change, but there are diverse points of view, so I guess we need to agree what the solution should be before working on code.

@lazylester I wasn't aware of the previous use of default_format in haml (pre haml 5 I'm guessing, but I haven't found a def default_format in v3 or v4). In any case the re-use of default_format for template resolution purposes was effectively only started in rails 6 I believe (with this commit: rails/rails#35404) so those earlier uses wouldn't have impacted anyone. [3]

And my point about this not being appropriate in haml, as it's only rails-related, is probably irrelevant as haml already contains a lot of rails-targeted code.

Here's my understanding of the situation. The work that default_format does is to change the way that templates are resolved (discovered by their name in the filesystem), by rails. Confusingly, the (response) format is determined by rails based on the request, and is not (AFAICT) adjusted by the "default_format" that the templating system defines (the work of default_format in the templating system does not seem to be documented anywhere in rails sadly , despite it being used by many templating plugins like haml). This response format could be any defined mime-type's symbol (html, js, json, csv... and/or a custom defined one).

The rails template resolver looks for a template matching the action.format.handler where handler is a Template Handler's extension (haml, erb, etc). Layout and partials are looked up using the same mechanism. (There's actually locales and variants as well, but I'm simplifying, see https://github.com/rails/rails/blob/8776e951f269dc161074aa75f76f115289e14c3a/actionview/lib/action_view/template/resolver.rb#L26-L36).

For partials and layouts, if the exact match doesn't exist, that's where default_format kicks in. Without a default_format, the resolver looks for (will match on) layout_or_partial.handler (a nil format in other words). But if the template defines a default_format then the resolver looks for (will match on) layout_or_partial.default_format.handler.

So haml defines their default_format as html, and the user of haml follows rails conventions, then they will have a surprise - a js format which uses haml will select the application.html.haml layout, not application.haml (or more likely, if they haven't defined any application.handler layout, skip the layout[1])

The answer is for everyone using formats which are not html with haml, to make sure to define layout application.js.handler (for example[2]) as empty, or say layout: false, and for partials to make sure there's an exact match. And if multiple non-html formats require exactly the same content (e.g. the edge case outlined in #1154) then you have to duplicate the files with each format.

It could be that the examples I've given here are edge cases and not important. But the options are:

• "(re)name your files with html.haml" (for haml to work well out of the box with Turbo-rails) or
• "reduce the functionality and break some people's existing code for existing haml users"

But i think all of this discussion is mostly about rails and how it behaves. Hence my feeling that this code belongs in rails or (as a monkey patch) in an app.

[1] the rule for layouts is -- no match, then skip the layout, whereas for partials, it's no match, then error.
[2] when I say handler here, I mean any of erb, haml, etc.
[3] The actual use for template resolution is (as of rails 7.1) in https://github.com/rails/rails/blob/8776e951f269dc161074aa75f76f115289e14c3a/actionview/lib/action_view/template_details.rb#L63 and https://github.com/rails/rails/blob/8776e951f269dc161074aa75f76f115289e14c3a/actionview/lib/action_view/unbound_template.rb#L55.

[lazylester]

good research @timdiggins.

I think there's a third option, going forwards, which is to deprecate the use of #default_format and announce the intention to remove it in a later version. Really this has been my point all along, that behaviour shouldn't just be dropped silently.

[timdiggins]

@lazylester unless I misunderstood, your third option seems to refer to how rails should work, not how haml works? Using the template default_format is a novelty in Haml 5 / 6, so "deprecating it" doesn't make sense I think (why introduce something in a patch release and then deprecate it).

I think the monkey patch approach would still work for people using Turbo-Rails (just only with RailsTemplate and without the Plugin pathc) and wouldn't cause any issues with existing Haml users who aren't using turbo (or not yet, or not exclusively).

Hence why I'm suggesting reverting this. Thoughts Les, or have I got the wrong end of the stick?

[lazylester]

The monkey patch works just fine. The problem I'm addressing, and the one I experienced, is that my app broke when I upgraded to the hamlit version of haml, because a feature of haml had been dropped. So I had to spend some time troubleshooting the internals of rails turbo to figure out why. Since it appears to me to be a pretty benign feature to restore, it seemed like an easy way to reduce the migration pain for other developers.

All your forensic work is interesting, but it doesn't address the problem that I, and evidently others too (from the rails issue discussion) have encountered.

[timdiggins]

All your forensic work is interesting, but it doesn't address the problem that I, and evidently others too (from the rails issue discussion) have encountered.

Ah, ok, yes, I wasn't very clear! My suggestion is to revert this commit, but also as part of that PR noting the requirement for turbo-rails compatibility in either the documentation or in the CHANGLEOG for Haml 6 breaking changes something like the following:

For people using Turbo-rails and Haml 6+ need to either:

• follow the naming convention with the html format (ie. ensure any .haml files end .html.haml), or

• add a monkey patch as follows:

   # config/initializers/haml.rb
   require "haml/rails_template"
  
   module Haml
     class RailsTemplate
       def default_format
         :html
       end
     end
   end
  

@lazylester What do you think? Would you agree to this change? If so, I'm happy to contribute the PR if you don't want to. But I wouldn't want to contribute the reversion PR without your agreement.

[lazylester]

documenting the problem is good, but not as good as a deprecation notice. But I'm still not understanding why inclusion of the default_format method is problematic. A js file with inline html, you said. Wasn't this problematic for haml prior to its merging with hamlit, when default_format was supported? Is there some framework other than turbo that is testing the value of default_format and getting the wrong result?

[k0kubun]

Thank you both, @timdiggins and @lazylester, for your thoughtful discussion on this issue.

haml has historically always supported #default_format

I wasn't aware of the previous use of default_format in haml (pre haml 5 I'm guessing, but I haven't found a def default_format in v3 or v4)

Using the template default_format is a novelty in Haml 5 / 6

Wasn't this problematic for haml prior to its merging with hamlit, when default_format was supported?

Let me get this straight. Here's the template_handler registered to Rails in v4 (no influence of Hamlit, which should have accumulated all historical code) and v5 (not entirely replaced by Hamlit yet, but migrated to Temple):

• v4.0.7: Haml::Plugin
• v5.2.2: Haml::Plugin

Neither of these defines #default_format, at least inside Haml. So, to my knowledge, haml.gem defining #default_format is completely a new feature in v6.1.3.

---

Since this is a fairly new feature, I believe reverting it is also fixes #1152 and #1154, and this was not consistent with the recommendation by DHH himself, I'll revert this as @timdiggins proposed, in a minor release as a semi-breaking bugfix.

To mitigate the impact for Turbo users, I'll cut a teeny release before that which only prints a deprecation warning in #default_format as @lazylester proposed, and commit #1144 (comment) as a documentation.