-
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
Defer loading types until they're needed #4974
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, but I think there might be some use-cases we should consider adding support for:
-
I'd like to support procs as well as strings for those using type checking in their projects. This makes it harder for to derive the name of a type before it is loaded, but it is somewhat easy to implement on top of this patch.
-
This patch currently eager loads all types when a query is executed, and I think we can do better. The current approach doesn't offer much benefit over plain old autoloading of a schema in a Rails app since you reference the constant when you make a query. It would be interesting to see if we could selectively eager load a specific root type depending on what field is being asked for in a query. I tried implementing a lightweight patch for this here but it doesn't cover type loading cases that don't have query contexts and seems a bit brittle.
-
When a schema is defined, you can use the
use
method to add an extension that might call thequery
,mutation
, orsubscription
method of the schema. With this patch, they are still strings (or procs if you apply my patch above), so I wonder if it would make sense to implement some kind of callback mechanism to wait until the appropriate root type is loaded and types added. -
I tried taking selective root type loading a step further by decoupling root type loading from type adding and traversal in gmcgibbon@ea86211, but we have internal API cases where we call
schema.query
andschema.mutation
in places like warden initialization. I'm not sure if these can be easily refactored, I'll have to look into it.
Thanks for sharing all this thoughtful feedback ... Still quite a few things to figure out 😅
I guess I was thinking that it would improve cases when you boot the app in development and use non-GraphQL features. But in that case, if your schema is autoloaded, you wouldn't've loaded it anyways. I guess I was thinking back to my old GitHub days, where the schema was always loaded at boot, regardless, but I think that was an application setup issue anyways 😖 . Yes, I can see how this would have to be pushed a little further to be useful for the normal case.
Maybe Just Works ™️ isn't an option -- maybe future-compat via callbacks is the way to go, like you suggested. Decoupling traversal would be huge -- I don't quite have a vision for how that'd really work yet, because given a query like |
The way I've always approached large refactors like this is to make it work first, and then make the backwards compatibility happen later. Once we have a better idea of all the changes we need to make, we can worry about compat IMO. Further on the warden type traversal problem I was mentioning, I found that the way we call this method makes it difficult to lazily load root types in general because we have to ask every type we query if it is a root type here. I'm switching gears and trying to make shallow schema type additions work like I mentioned here. This way, we can keep the way type resolution works and eliminate the root problem of a root type needing to know about all of its nested types when it is loaded. Thank you so much for your help with this, I really appreciate it! |
That |
This was a worthwhile exercise (for me anyways) but not the right step forward, so I'm closing it 🍻 |
GraphQL-Ruby could reduce application boot time by not loading the whole schema upfront. Instead, it could load the schema as-needed.
To opt into lazy-loaded types:
Add
load_types_as_needed
to the top of your Schema class (before any types are configured), for example:Use strings to identify your root types (
query
,mutation
, and/orsubscription
) andorphan_types
, using fully-qualified Ruby constant names. For example:(If you use Ruby constant literals instead of strings, then Rails will load the constants right away. When you use strings, GraphQL-Ruby will
Object.const_get(...)
the type definition when it needs it.)At a minimum, it could load the whole schema the first time it's used. Ideally, it would load only the bits of schema needed for a given query. (Some queries would require the whole schema to be loaded.)
On the other hand, it's better for a production server to have everything loaded once, and before any requests are received (and before any
fork
ing, to avoid duplicates in child process memory).TODO:
Schema
state not being loaded until necessary (maybe add anunload
-type method for testing)Identify calls toI think this can be a second stepSchema.
methods which requirebuild_types_hash
, reduce usage of themGraphQL::Schema
so that root types and orphan types can be registered without causing those files to be loaded in RailsUpdatewill address in Schema type system clean-up #4975possible_types
to use type classes, not strings, as keysFixes #4919