-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use config.before_eager_load #5182
Conversation
We want to make sure the gem is fully loaded in a production context before the application begins eagerly loading, otherwise we emit a warning. So, let's use config.before_eager_load to make sure gem eager loading happens before the app.
Awesome, thanks! |
Ugh, sorry 😖 Yes, to help debug, It'd be great to get a backtrace when that warning is created. That would show exactly what part of Rails caused GraphQL::Schema to be loaded. Here's a branch that will print the backtrace when that warning happens: #5185 Does it print if you start the rails server locally? Could you bundle that version and share the backtrace? For example: gem "graphql", github: "rmosolgo/graphql-ruby", ref: "debug-eager-load-warning" (You could also |
@rmosolgo Here's what I see in an app I'm working on using the
The app is using the
If I remove I don't know enough about the internals of this gem or the graphql-client gem to confidently say which is more responsible for triggering the warning. But! I wanted to share the experience in hopes that it helps sort out a fix or enhancement. |
Ideally, if GraphQL client gem uses parts of this GraphQL gem, it would make sense for it to also be autoloaded. As for #5182 (comment), I would suspect something similar like a gem dependency is loading GraphQL before we expect it to. I need a backtrace to verify, though. The problem mainly stems from the warning assuming that the schema will be loaded by the app, but it could be loaded by any dependent library at bundler require time. Most gems don't bother with this warning, and this may be part of why. We don't control what loads our code in consuming apps, and the worst case scenario is in more exotic use-cases, we don't load as much as we really should upfront (but it still works). Happy to submit a patch to the client in the meantime. |
Yeah, I'm inclined to remove this warning since I can't exactly nail down when it should be logged: #5187 What do you think, @gmcgibbon ? |
I agree removing it is probably the least error-prone and more future proof) approach. Let's remove it. |
I removed it altogether and shipped 2.4.7. Thanks for your help tracking this down, everyone, and sorry the trouble along the way! |
We want to make sure the gem is fully loaded in a production context before the application begins eagerly loading, otherwise we emit a warning. So, let's use
config.before_eager_load
to make sure gem eager loading happens before the app.Before:
After: