-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
Feature Request/Unexpected behaviour: Graphql handlers just return string ids instead of relationships #99
Comments
Hey, @phryneas. You're right, the current GraphQL type of a relation would be reduced to a primitive, which doesn't produce a valid GraphQL schema in the end: factory({
user: {
post: oneOf('post')
},
post: { ... }
}) type User {
- post: String
+ post: Post
} I can't say from the top of my head if the schema generation logic can determine a value being a relational node to infer its type, or we'd have to extend the logic to have access to relational nodes. Please, would you be interested in tackling this? I'd be happy to help with code reviews in the process. |
I don't really have the time for that as I have tons of open issues to work on myself :/ But if I find one or two hours I'll dig into the code - maybe it's less work than I'm imagining. But otherwise, if someone else sees this issue and thinks "Hey, I can do this" - take it! :D I don't really need this right now (although it would be nice in our examples), I just noticed it and wanted to put it here as a suggestion for the roadmap - with whichever priority you give it in the end :) |
Since a model may reference other models (relational properties), it's no longer possible to generate a complete GraphQL schema from a single given model (it may depend on other models). In other words, given a factory like this: const db = factory({
user: {
id: primaryKey(datatype.uuid),
role: oneOf('role'),
},
role: {
id: primaryKey(datatype.uuid),
name: String,
},
}) Generating the GraphQL types via As I've suggested above, it looks like the library needs to have access to the entire dictionary to generate a complete GraphQL schema for any given model. At first, I thought about optimizing the generation process by analyzing model dependencies a single dependency has (what models it references, like To summarize, it looks like this change implies the internal change: -generateGraphQLSchema(model): GraphQLSchema
# We'd have to expose the dictionary and generate GraphQL types for all models.
+generateGraphQLSchema(model, dictionary): GraphQLSchema |
I just noticed that over in #96 while adding tests.
If I were to add a second model "Post" with a
oneOf
relationship "author" to "User", that would still be output asString
instead of referencing theUser
type, which would be expected of a graphql api.This is not a problem of my other PR, but a conceptual problem with
toHandlers
. It would probably have to be part ofFactory
instead of being part ofModel
, to handle these spanning relationships and quite some additional work ongetGraphQLType
, which is very simple right now:data/src/model/generateGraphQLHandlers.ts
Lines 35 to 45 in f8da857
The text was updated successfully, but these errors were encountered: