Use primary key fields if available for the @key directive #2
Conversation
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 |
| ? 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