Add turbo_frame_dom_id helper

[skipkayhil]

This enables being able to pass the generated id of a turbo_frame_tag to turbo_stream actions.

For example:

<%# app/views/articles/show.html.erb %>
<%= turbo_frame_tag(Article.find(1), "comments") %>

<%# app/views/comments/create.turbo_stream.erb %>
<%= turbo_stream.append turbo_frame_dom_id(Article.find(1), "comments"), @comment %>

Using dom_id in this case does not work, because the turbo_frame appends additional arguments to the id while dom_id prepends them.

[marcoroth]

I'm wondering if we instead should change the way the turbo frame helper generates the ID

[packagethief]

I'm wondering if we instead should change the way the turbo frame helper generates the ID

I agree with @marcoroth. turbo_frame_tag is already using dom_id under the covers, and the result is consistent when called with a single argument. But unlike turbo_frame_tag, dom_id doesn't accept variable arguments. It accepts two, the second of which is the prefix:

dom_id(@recording, :comments) #=> "comments_recording_123"

turbo_frame_tag does accept variable arguments, and there's no way to provide a prefix. We just get the objects in order. That's not intentional. The intent is for it to behave exactly like dom_id, so it's easy to use and unsurprising, as the documentation suggests:

turbo-rails/app/helpers/turbo/frames_helper.rb

Lines 27 to 28 in 728b963

	# The `turbo_frame_tag` helper will convert the arguments it receives to their
	# `dom_id` if applicable to easily generate unique ids for Turbo Frames:

My preference would be to change turbo_frame_tag so that its first positional arguments are passed directly dom_id, and remove support for passing multiple records or identifiers.

The downside, of course, is that it's a breaking change for anyone using it with multiple positional arguments and expecting a specific result. I would argue that the behavior is already broken – it's impossible to generate a dom_id-compatible turbo frame tag with multiple identifiers or with a specific prefix. It seems worth the effort, though perhaps in a major release. As long as we're clear about the difference in release notes, it should be fine.

/cc @afcapel, @kevinmcconnell

[davidalejandroaguilar]

Yes!

This was one of the many things that took me by surprise when trying Hotwire. Had planned to open an issue but hadn't had time to do so.

I agree that turbo_frame_tag should rather be fixed than implementing what this PR suggests.

[skipkayhil]

Changing to just use dom_id works for me. Would a configuration option in the Engine be a reasonable way to change/deprecate?

Something roughly like

module Turbo
  class Engine < Rails::Engine
    # ...

    config.turbo.use_dom_id_for_frames = false
    
    config.after_initialize do
      if config.turbo.use_dom_id_for_frames
        # include new id generation method in helper
      else
        Turbo.deprecator.warn "deprecated"
        # include old id generation method in helper
      end
    end
  end
end

[dhh]

I consider the current setup a bug. Should just fix to use dom_id directly. Feel free to open a PR for that.