Add support for Hotwire Turbo Drive

[ceritium]

It is related to this issue #482

I moved to the gem the @mildred's solution.

I tested it on a small demo app, as I show in the following video:

turbodrive.support.demo.mov

Any recommendation for better QA?

[Dainii]

Tested with rails (6.1.3.1) and turbo-rails (0.5.9), it seems to work quite nicely.

[kbrock]

This looks great. but please remember:

1. I am not a rack-mini-profiler developer. I do not have merge rights. (feel free to ignore my suggestions)
2. I am not a ui developer and just looking at the code and suggesting you copy stuff.

[SamSaffron]

This looks right to me @OsamaSayegh should we merge?

[codecov-commenter]

Codecov Report

Merging #498 (2826fc6) into master (a93060e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #498   +/-   ##
=======================================
  Coverage   87.50%   87.50%           
=======================================
  Files          18       18           
  Lines        1257     1257           
=======================================
  Hits         1100     1100           
  Misses        157      157           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a93060e...2826fc6. Read the comment docs.

[OsamaSayegh]

Thank you for this PR @ceritium ❤️ I'd love to merge this, but I have a couple of comments:

Re:

Doing the Turbo constant check avoid adding unnecessary event listeners, but it requires that the application exposes Turbo globally:

import { Turbo } from "@hotwired/turbo-rails"
window.Turbo = Turbo

What do you think? What is the best approach? I guess we should mention it in the documentation.

Requiring people to do something like that feels too much magical to me 😅 Instead, I'd prefer if we added a new enable_hotwire_turbo_support config option (false by default) to lib/mini_profiler/config.rb that people who use Hotwire in their application can turn on. When it's turned on, we can use that to signal to MiniProfiler to add Hotwrite's specific bits.

For example, in lib/html/profile_handler.js I'd replace the data-turbo-permanent="true" attribute that you have now with a new placeholder {hotwriteTurboSupport}. The get_profile_script method here:

rack-mini-profiler/lib/mini_profiler/profiler.rb

Lines 720 to 774 in a93060e

	def get_profile_script(env)
	path = "#{env['RACK_MINI_PROFILER_ORIGINAL_SCRIPT_NAME']}#{@config.base_url_path}"
	version = MiniProfiler::ASSET_VERSION
	if @config.assets_url
	url = @config.assets_url.call('rack-mini-profiler.js', version, env)
	css_url = @config.assets_url.call('rack-mini-profiler.css', version, env)
	end
	url = "#{path}includes.js?v=#{version}" if !url
	css_url = "#{path}includes.css?v=#{version}" if !css_url
	content_security_policy_nonce = @config.content_security_policy_nonce ||
	env["action_dispatch.content_security_policy_nonce"] ||
	env["secure_headers_content_security_policy_nonce"]
	settings = {
	path: path,
	url: url,
	cssUrl: css_url,
	version: version,
	verticalPosition: @config.vertical_position,
	horizontalPosition: @config.horizontal_position,
	showTrivial: @config.show_trivial,
	showChildren: @config.show_children,
	maxTracesToShow: @config.max_traces_to_show,
	showControls: @config.show_controls,
	showTotalSqlCount: @config.show_total_sql_count,
	authorized: true,
	toggleShortcut: @config.toggle_shortcut,
	startHidden: @config.start_hidden,
	collapseResults: @config.collapse_results,
	htmlContainer: @config.html_container,
	hiddenCustomFields: @config.snapshot_hidden_custom_fields.join(','),
	cspNonce: content_security_policy_nonce,
	}
	if current && current.page_struct
	settings[:ids] = ids_comma_separated(env)
	settings[:currentId] = current.page_struct[:id]
	else
	settings[:ids] = []
	settings[:currentId] = ""
	end
	# TODO : cache this snippet
	script = IO.read(::File.expand_path('../html/profile_handler.js', ::File.dirname(__FILE__)))
	# replace the variables
	settings.each do |k, v|
	regex = Regexp.new("\\{#{k.to_s}\\}")
	script.gsub!(regex, v.to_s)
	end
	current.inject_js = false if current
	script
	end

is what replaces the placeholders in profile_handler.js with the right values. So, I'd add a check for our new config, and if it's true I'd set hotwriteTurboSupport in the settings hash to data-turbo-permanent="true"; otherwise set hotwriteTurboSupport to the empty string.

Finally, instead of checking for the Turbo constant on the client side, we can check for the data-turbo-permanent attribute on MiniProfiler's <script> tag and add our event listeners if the attribute exists.

The advantage of this is it allows to conditionally add the data-turbo-permanent="true" attribute only for the people who use Turbo, and avoid magical hacks like window.Turbo = Turbo.

One last thing, could you run the update_asset_version rake task when you're done? This updates the ASSET_VERSION constant which we use to bust cache when new versions are released.

Let me know if you have any questions!

[ceritium]

Thanks for your feedback @OsamaSayegh. I like your approach. I just pushed a commit addressing your feedback and a small test.

Would you add more tests?
Do you have any suggestions about variable naming?
It's ok the place for enable_hotwire_turbo_drive_support on "config.rb" or would you move it to a different place?

[OsamaSayegh]

I think it looks great, thank you @ceritium!

Edit: One more thing; could you please document the new config in table the README file? https://github.com/MiniProfiler/rack-mini-profiler#configuration-options

[ceritium]

@OsamaSayegh I added the documentation.

Do you like the commits as they are? Would you squash them? I didn't found any policy about that.

[OsamaSayegh]

Perfect, thank you for all your work on this!

Github has a merge option that squashes everything together which I'm going to use now!