Skip to content
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

[new rule] validate subgraph according federation 2 #2049

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`federation-subgraph > invalid > should validate subgraph 1`] = `
#### ⌨️ Code

1 | type Query {
2 | t: T
3 | }
4 |
5 | type T @key(fields: "f") {
6 | f(x: Int): Int
7 | }

#### ❌ Error

> 1 | type Query {
| ^^^^^^^^^^^^^^^^^^^^
> 2 | t: T
| ^^^^^^^^^^^^^^
> 3 | }
| ^^^^^^^^^^^^^^
> 4 |
| ^^^^^^^^^^^^^^
> 5 | type T @key(fields: "f") {
| ^^^^^^^^^^^^^^
> 6 | f(x: Int): Int
| ^^^^^^^^^^^^^^
> 7 | }
| ^^^^^^^^^^ On type "T", for @key(fields: "f"): field T.f cannot be included because it has arguments (fields with argument are not allowed in @key)
`;
22 changes: 22 additions & 0 deletions packages/plugin/__tests__/federation-subgraph.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { rule } from '../src/rules/federation-subgraph';
import { ruleTester } from './test-utils';

ruleTester.run('federation-subgraph', rule, {
valid: [],
invalid: [
{
// https://github.com/apollographql/federation/blob/4ffe723395b4a054807b4f58181f166d76b57160/internals-js/src/__tests__/subgraphValidation.test.ts#L9
name: 'should validate subgraph',
code: /* GraphQL */ `
type Query {
t: T
}

type T @key(fields: "f") {
f(x: Int): Int
}
`,
errors: 1,
},
],
});
8 changes: 8 additions & 0 deletions packages/plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,15 @@
"typecheck": "tsc --noEmit"
},
"peerDependencies": {
"@apollo/federation-internals": "^2",
"eslint": ">=8.44.0",
"graphql": "^16"
},
"peerDependenciesMeta": {
"@apollo/federation-internals": {
"optional": true
}
},
Comment on lines +46 to +50
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think people should explicitly install this dependency if they want to validate federation schema, there is no need to add to dependencies

"dependencies": {
"@graphql-tools/code-file-loader": "^8.0.0",
"@graphql-tools/graphql-tag-pluck": "^8.0.0",
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should add @graphql-tools/graphql-tag-pluck also to peerDependencies in v4, since it's a big dependency that installs a lot of things (not everybody lint code files)

Expand All @@ -53,6 +59,7 @@
"lodash.lowercase": "^4.3.0"
},
"devDependencies": {
"@apollo/federation-internals": "^2.6.1",
"@theguild/eslint-rule-tester": "workspace:*",
"@types/debug": "4.1.12",
"@types/eslint": "8.44.8",
Expand All @@ -61,6 +68,7 @@
"@types/json-schema": "7.0.15",
"@types/lodash.lowercase": "4.3.9",
"graphql": "16.8.1",
"graphql-tag": "^2.12.6",
"json-schema-to-ts": "2.9.2"
},
"publishConfig": {
Expand Down
64 changes: 32 additions & 32 deletions packages/plugin/src/estree-converter/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,34 +79,34 @@ type NodeWithType =
type ParentNode<T> = T extends DocumentNode
? AST.Program
: T extends DefinitionNode
? DocumentNode
: T extends EnumValueDefinitionNode
? EnumTypeDefinitionNode | EnumTypeExtensionNode
: T extends InputValueDefinitionNode
?
| DirectiveDefinitionNode
| FieldDefinitionNode
| InputObjectTypeDefinitionNode
| InputObjectTypeExtensionNode
: T extends FieldDefinitionNode
?
| InterfaceTypeDefinitionNode
| InterfaceTypeExtensionNode
| ObjectTypeDefinitionNode
| ObjectTypeExtensionNode
: T extends SelectionSetNode
? ExecutableDefinitionNode | FieldNode | InlineFragmentNode
: T extends SelectionNode
? SelectionSetNode
: T extends TypeNode
? NodeWithType
: T extends NameNode
? NodeWithName
: T extends DirectiveNode
? InputObjectTypeDefinitionNode | ObjectTypeDefinitionNode
: T extends VariableNode
? VariableDefinitionNode
: unknown; // Explicitly show error to add new ternary with parent nodes
? DocumentNode
: T extends EnumValueDefinitionNode
? EnumTypeDefinitionNode | EnumTypeExtensionNode
: T extends InputValueDefinitionNode
?
| DirectiveDefinitionNode
| FieldDefinitionNode
| InputObjectTypeDefinitionNode
| InputObjectTypeExtensionNode
: T extends FieldDefinitionNode
?
| InterfaceTypeDefinitionNode
| InterfaceTypeExtensionNode
| ObjectTypeDefinitionNode
| ObjectTypeExtensionNode
: T extends SelectionSetNode
? ExecutableDefinitionNode | FieldNode | InlineFragmentNode
: T extends SelectionNode
? SelectionSetNode
: T extends TypeNode
? NodeWithType
: T extends NameNode
? NodeWithName
: T extends DirectiveNode
? InputObjectTypeDefinitionNode | ObjectTypeDefinitionNode
: T extends VariableNode
? VariableDefinitionNode
: unknown; // Explicitly show error to add new ternary with parent nodes

type Node<T extends ASTNode, WithTypeInfo extends boolean> =
// Remove readonly for friendly editor popup
Expand All @@ -130,7 +130,7 @@ export type GraphQLESTreeNode<T, W extends boolean = false> =
: GraphQLESTreeNode<Node<T, W>[K], W>;
}
: // If Program node => add `parent: null` field
T extends AST.Program
? T & { parent: null }
: // Return value as is
T;
T extends AST.Program
? T & { parent: null }
: // Return value as is
T;
62 changes: 62 additions & 0 deletions packages/plugin/src/rules/federation-subgraph.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import {
asFed2SubgraphDocument,
buildSchemaFromAST,
FederationBlueprint,
Subgraph,
} from '@apollo/federation-internals';
import { GraphQLESLintRule } from '../types.js';

const RULE_ID = 'federation-subgraph';
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any suggestions to rule name?


export const rule: GraphQLESLintRule = {
meta: {
type: 'problem',
docs: {
category: 'Schema',
description: 'Enforce subgraph to be valid according Federation 2 spec.',
url: `https://the-guild.dev/graphql/eslint/rules/${RULE_ID}`,
// examples: [
// {
// title: 'Incorrect',
// code: /* GraphQL */ ``,
// },
// {
// title: 'Correct',
// code: /* GraphQL */ ``,
// },
// ],
},
schema: [],
},
create(context) {
return {
Document(node) {
// https://github.com/apollographql/federation/blob/4ffe723395b4a054807b4f58181f166d76b57160/internals-js/src/__tests__/testUtils.ts#L16C45-L16C67
const doc = asFed2SubgraphDocument(node.rawNode());

const withRootTypeRenaming = true;
const buildOptions = {
blueprint: new FederationBlueprint(withRootTypeRenaming),
validate: false,
};

// https://github.com/apollographql/federation/blob/4ffe723395b4a054807b4f58181f166d76b57160/internals-js/src/federation.ts#L1329
const subgraphSchema = buildSchemaFromAST(doc, buildOptions);

const name = 'graphql-eslint-subgraph';

const subgraph = new Subgraph(name, `http://${name}`, subgraphSchema);

try {
subgraph.validate();
} catch (error) {
context.report({
node,
// Remove `[graphql-eslint-subgraph] ` suffix in error message
message: (error as Error).message.slice(name.length + '[] '.length),
});
}
},
};
},
};