conditionally require graphql/dashboard#5515
Conversation
|
Thanks for working on this fix! This change will resolve the immediate issue we encountered with rambulance (and likely other gems using However, I have a concern about using Potential IssueWith
This is essentially the same issue, just triggered by a different gem instead of graphql itself. Suggested AlternativeUsing a Railtie # graphql.rb
if defined?(::Rails::Engine)
autoload :Dashboard, 'graphql/dashboard'
end
# graphql/railtie.rb
initializer 'graphql.setup_dashboard', before: :add_routing_paths do
require 'graphql/dashboard'
endThis ensures the Dashboard is loaded at a specific point in Rails' initialization (after Zeitwerk is configured), regardless of when other gems might load That said, I understand this change is simpler and solves the immediate problem. Just wanted to flag this potential edge case for consideration. |
|
👋 I'm definitely interested in fixing this, but I don't know how to replicate the problem. Could one of you share a script that demonstrates the problem? For example, some @hotoolong, I took a crack at that approach in #5523 and found that tests failed in some strange way (I don't really understand it -- I think those are the same failures that resulted in the code that's currently there.) I also took another approach in #5524 which might also solve this problem in another way. Please let me know if you can share any details about how I might check for this problem being solved. Also, feel free to try any of those other branches. I'd like to add a test to graphql-ruby before merging one of them, but you can let me know if either of them work for you. |
|
@rmosolgo Thanks for taking the time to review my PR. Here is the script i'm using to detect this I've tried both PRs, they still autoloaded constants for me at bootup # frozen_string_literal: true
# Extracted and adapted from this talk from Ben Sheldon:
# `An ok compromise. Faster development by designing for the Rails Autoloader`
# Youtube video link: https://youtu.be/9-PWz9nbrT8?si=Lw7qsF2_VmBperId&t=1487
require_relative "../../config/application"
autoloaded_constants = []
Rails.autoloaders.each do |loader|
loader.on_load do |cpath, _value, _abspath|
autoloaded_constants << [cpath, caller]
end
end
Rails.application.initialize!
autoloaded_constants.each do |x|
x[1] = Rails.backtrace_cleaner.clean(x[1]).first
end
allow_listed_constants = %w[
HeicPreviewer
ActionText::ContentHelper
Statesman::Adapters::CustomActiveRecord
]
puts
if autoloaded_constants.reject! { _1.first.in?(allow_listed_constants) }.any?
puts
puts "ERROR: Autoloaded constants were referenced during during boot."
puts
puts "These files/constants were autoloaded during the boot process, which will result in" \
"inconsistent behavior and will slow down and may break development mode. " \
"Remove references to these constants from code loaded at boot. "
puts
puts
w = autoloaded_constants.map { _1.first.length }.max
autoloaded_constants.each do |name, location|
puts "`#{name.ljust(w)}` referenced by #{location}"
end
exit 1
else
puts "SUCCESS! No autoloaded constants were found during the boot process."
exit 0
end |
|
Thanks for trying the I was wrong about how Looking at #5524, I think that approach is more robust. By separating the controllers into their own files with The concern I have with |
|
Thanks for sharing that script, @chaadow. I adapted it to the dummy application in this gem's tests in 86ad4b9. However, in that scenario, the script succeeds -- no autoloaded constants are loaded during One possibility would be to dump the whole backtrace instead of the just first line (which points to the script itself). What if you delete this code: - autoloaded_constants.each do |x|
- x[1] = Rails.backtrace_cleaner.clean(x[1]).first
- endAnd re-run it locally. What backtraces do you get for those autoloaded constants? I bet they're very similar. If you can share a couple of them, it might help us track down what exactly is causing them to be loaded in your app (but not in mine). |
|
I just updated #5524 to also use If it's still not working properly for you, please share some more debugging info (perhaps along the lines of my suggestion above) and we can keep hunting this issue! I'm going to close this PR for now but happy to keep discussing here. |
|
@rmosolgo thank you very much, i've tested the master branch with your PR and it works, there is no longer any turbo-rails constants, since |
|
Glad to hear it! Thanks for letting me know. |
More context here: https://github.com/rmosolgo/graphql-ruby/pull/5505/files#r2725942442
Using this script, this require has triggered a lot of autoloaded constants related to ActionController
