Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Sylvain Lebresne committed Mar 25, 2022
1 parent d176f93 commit 7257cd2
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 6 deletions.
6 changes: 5 additions & 1 deletion composition-js/src/merging/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1787,7 +1787,11 @@ class Merger {
const element = e.extensions['inaccessible_element'];
// We only point to subgraphs where the element is marked inaccesssible, as other subgraph are not relevant.
const elementNodes = element && typeof element === 'string'
? sourceASTs(...this.subgraphsSchema.map((schema) => schema.elementByCoordinate(element)).filter((elt) => elt?.hasAppliedDirective(federationMetadata(elt.schema())!.inaccessibleDirective())))
? sourceASTs(
...this.subgraphsSchema
.map((schema) => schema.elementByCoordinate(element))
.filter((elt) => elt?.hasAppliedDirective(federationMetadata(elt.schema())!.inaccessibleDirective()))
)
: [];

// Note that the error we get will likely have some AST nodes, but those will be to the supergraph and that's not
Expand Down
2 changes: 1 addition & 1 deletion docs/source/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ The following errors may be raised by composition:
| `PROVIDES_INVALID_FIELDS` | The `fields` argument of a `@provides` directive is invalid (it has invalid syntax, includes unknown fields, ...). | 2.0.0 | |
| `PROVIDES_ON_NON_OBJECT_FIELD` | A `@provides` directive is used to mark a field whose base type is not an object type. | 2.0.0 | |
| `PROVIDES_UNSUPPORTED_ON_INTERFACE` | A `@provides` directive is used on an interface, which is not (yet) supported. | 2.0.0 | |
| `REFERENCED_INACCESSIBLE` | An element is marked as @inaccessible but is referenced by a non-inacessible elemnent (at least in post-merging of the subgraphs) | 2.0.0 | |
| `REFERENCED_INACCESSIBLE` | An element is marked as @inaccessible but is referenced by a non-inaccessible element. | 2.0.0 | |
| `REQUIRED_ARGUMENT_MISSING_IN_SOME_SUBGRAPH` | An argument of a field or directive definition is mandatory in some subgraphs, but the argument is not defined in all subgraphs that define the field or directive definition. | 2.0.0 | |
| `REQUIRES_FIELDS_HAS_ARGS` | The `fields` argument of a `@requires` directive includes a field defined with arguments (which is not currently supported). | 2.0.0 | |
| `REQUIRES_FIELDS_MISSING_EXTERNAL` | The `fields` argument of a `@requires` directive includes a field that is not marked as `@external`. | 0.x | |
Expand Down
7 changes: 6 additions & 1 deletion internals-js/src/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1407,6 +1407,11 @@ export class Schema {
return this.getBuiltInDirective(schema, 'specifiedBy');
}

/**
* Gets an element of the schema given its "schema coordinate".
*
* Note that the syntax for schema coordinates is the one from the upcoming GraphQL spec: https://github.com/graphql/graphql-spec/pull/794.
*/
elementByCoordinate(coordinate: string): NamedSchemaElement<any, any, any> | undefined {
if (!coordinate.match(coordinateRegexp)) {
throw error(`Invalid argument "${coordinate}: it is not a syntactically valid graphQL coordinate."`);
Expand All @@ -1418,7 +1423,7 @@ export class Schema {
const argName = argStartIdx < 0 ? undefined : coordinate.slice(argStartIdx + 1, coordinate.length - 2);
const splittedStart = start.split('.');
const typeOrDirectiveName = splittedStart[0];
const fieldOrEnumName = splittedStart.length > 1 ? splittedStart[1] : undefined;
const fieldOrEnumName = splittedStart[1];
const isDirective = typeOrDirectiveName.startsWith('@');
if (isDirective) {
if (fieldOrEnumName) {
Expand Down
3 changes: 1 addition & 2 deletions internals-js/src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,10 +346,9 @@ const LINK_IMPORT_NAME_MISMATCH = makeCodeDefinition(

const REFERENCED_INACCESSIBLE = makeCodeDefinition(
'REFERENCED_INACCESSIBLE',
'An element is marked as @inaccessible but is referenced by a non-inacessible elemnent (at least in post-merging of the subgraphs)'
'An element is marked as @inaccessible but is referenced by a non-inaccessible element.'
);


const REQUIRED_ARGUMENT_MISSING_IN_SOME_SUBGRAPH = makeCodeDefinition(
'REQUIRED_ARGUMENT_MISSING_IN_SOME_SUBGRAPH',
'An argument of a field or directive definition is mandatory in some subgraphs, but the argument is not defined in all subgraphs that define the field or directive definition.'
Expand Down
2 changes: 1 addition & 1 deletion query-graphs-js/src/querygraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ function buildGraphInternal(name: string, schema: Schema, addAdditionalAbstractT
const builder = new GraphBuilderFromSchema(
name,
schema,
supergraphSchema ? { apiSchema: supergraphSchema?.toAPISchema(), isFed1: isFed1Supergraph(supergraphSchema) } : undefined,
supergraphSchema ? { apiSchema: supergraphSchema.toAPISchema(), isFed1: isFed1Supergraph(supergraphSchema) } : undefined,
);
for (const rootType of schema.schemaDefinition.roots()) {
builder.addRecursivelyFromRoot(rootType.rootKind, rootType.type);
Expand Down

0 comments on commit 7257cd2

Please sign in to comment.