Refactor query reading to use graphql-anywhere #747
Conversation
336561c to
8dda7fe
Compare
|
747 |
|
Nice, so much complicated code removed! I will be even simpler once we stop converting from document to AC representation and back 😃 |
| ApolloError, | ||
| } from '../errors/ApolloError'; | ||
|
|
||
| import graphql, { |
There was a problem hiding this comment.
So many things called graphql now... I guess that's life.
There was a problem hiding this comment.
I'll change to graphql as graphqlAnywhere just to keep it classy.
| fieldName: string, | ||
| objId: string, | ||
| args: any, | ||
| context: ReadStoreContext |
There was a problem hiding this comment.
are these the standard arguments to graphql-anywhere resolvers? Can you walk me through graphql-anywhere execution some time, so I understand why fieldName and objectId are there?
There was a problem hiding this comment.
Also, I would have expected to somehow get the parent somehow, but apparently that's not how it works?
There was a problem hiding this comment.
In graphql-anywhere, it's fieldName, rootValue, args, context. In this case, the rootValue just happens to be the object ID because that's what the parent resolver returns.
There was a problem hiding this comment.
Ok, I see. I would have expected the parent to be the first argument. Maybe fieldName could be the last, since it's kind of like the info field in graphql?
There was a problem hiding this comment.
I guess it's the first argument because in regular GraphQL it's the first thing you write:
Query: {
myFieldName: (root, args, context) => { ... }
}In this case it's: (myFieldName, root, args, context) => { ... }
| if (throwOnMissingField) { | ||
| handleFragmentErrors(fragmentErrors); | ||
| } | ||
| const result = graphql(readStoreResolver, doc, 'ROOT_QUERY', context, variables, mapper); |
There was a problem hiding this comment.
we could one-up graphql here and make the arguments named. (i know that's technically in graphql-anywhere, but I'm just putting all my thoughts here).
There was a problem hiding this comment.
What is the role of ROOT_QUERY here? It's kind of like a rootValue, but the resolvers don't return field names, so I'm unsure of how it works.
There was a problem hiding this comment.
The current implementation has the resolvers return either the ID of the related object or a scalar leaf field. So the initial root value is the ID of the root object, which is ROOT_QUERY.
| @@ -88,9 +88,13 @@ export function storeKeyNameFromField(field: Field, variables?: Object): string | |||
| } | |||
|
|
|||
| export function storeKeyNameFromFieldNameAndArgs(fieldName: string, args?: Object): string { | |||
| const stringifiedArgs: string = JSON.stringify(args); | |||
| if (args) { | |||
| const stringifiedArgs: string = JSON.stringify(args); | |||
There was a problem hiding this comment.
wait, did we never do anything about alphabetical ordering etc? Because we'll definitely get a cache miss if the order isn't the same, even if the values are semantically equivalent.
There was a problem hiding this comment.
Yes, we might want to do that. Perhaps something to open a new issue about, since it doesn't necessarily need to be part of the refactor.
Part of #728
First step towards #617
TODO:
Still to do: