Use primary key fields if available for the @key directive#2
Use primary key fields if available for the @key directive#2jgzuke wants to merge 4 commits intographile:mainfrom
Conversation
benjie
left a comment
There was a problem hiding this comment.
I’m happy merging something like this, but you also need to add support for this selection set to the _entities field which currently uses resolveNode (the node interface) to load the data. I think you’d need to use selectGraphQLResultsFromTable instead, figuring out the table from the __typename, and using the new build.scopeByType lookup (something like scopeByType(getTypeByName(__typename)).pgIntrospection).
|
@benjie I couldnt find |
benjie
left a comment
There was a problem hiding this comment.
This is an excellent start! Sorry for the delay getting back to you, July was incredibly busy but I'm spending August catching up so responsiveness should be better soon :) Hopefully the comments in this review are useful.
| ? primaryKeyNames.join(" ") | ||
| : nodeIdFieldName; | ||
|
|
||
| astNode.directives.push(Directive("key", { fields: StringValue(keyName) })); |
There was a problem hiding this comment.
We should definitely support the nodeId by default; using the primary keys directly is not a best practice. Adding support for primary keys is desired if the node plugin is disabled, and it could also be used as an alternate fetcher. Fortunately according to the Apollo federation spec you can provide @key multiple times; so this is what we should do - once for nodeId, and once for the PKs.
| } = scopeByType.get(type); | ||
| const identifiers = attrs.map( | ||
| attr => representation[inflection.column(attr)] | ||
| ); |
There was a problem hiding this comment.
Looks good until here, but from here on we want to query in a way that will work even if we don't have the NodePlugin loaded. You should have access to resolveInfo.graphile.selectGraphQLResultsFromTable which you can read about here:
If there are a set of
primaryKeyConstraints for a given resource, use those for the federation key.If there is no
primaryKeyConstraint, use thenodeIdsame as before.See #1 for more explanation