Skip to content

Commit

Permalink
Enforce require-selections on FragmentSpreads within `GraphQLUnio…
Browse files Browse the repository at this point in the history
…nType`s (#2319)

* Add failing test

* Support fragment spreads in union types in `require-selections`

* Add changeset

* upd snapshot

* more

* more

* more

* more

---------

Co-authored-by: Dimitri POSTOLOV <[email protected]>
  • Loading branch information
maciesielka and dimaMachina authored Nov 16, 2024
1 parent 0bc742b commit b3c73dc
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 3 deletions.
4 changes: 3 additions & 1 deletion .changeset/empty-horses-relate.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@
'@graphql-eslint/eslint-plugin': patch
---

fix false positive cases for `require-import-fragment` on Windows, when `graphql-config`'s `documents` key contained glob pattern => source file path of document contained always forward slashes
fix false positive cases for `require-import-fragment` on Windows, when `graphql-config`'s
`documents` key contained glob pattern => source file path of document contained always forward
slashes
2 changes: 1 addition & 1 deletion .changeset/empty-singers-develop.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
"@graphql-eslint/eslint-plugin": minor
'@graphql-eslint/eslint-plugin': minor
---

feat: add a new option `{` for alphabetize rule to sort fields `selection set`
5 changes: 5 additions & 0 deletions .changeset/twenty-tables-help.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-eslint/eslint-plugin': major
---

Enforce `require-selections` on `FragmentSpread`s within `GraphQLUnionType`s
71 changes: 71 additions & 0 deletions packages/plugin/src/rules/require-selections/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,19 @@ const TEST_SCHEMA = /* GraphQL */ `
noId: NoId!
vehicles: [Vehicle!]!
flying: [Flying!]!
noIdOrNoId2: NoIdOrNoId2!
}
type NoId {
name: String!
}
type NoId2 {
name: String!
}
union NoIdOrNoId2 = NoId | NoId2
interface Vehicle {
id: ID!
}
Expand Down Expand Up @@ -310,6 +317,22 @@ ruleTester.run<RuleOptions, true>('require-selections', rule, {
...WITH_SCHEMA,
code: '{ hasId { id: name } }',
},
{
name: 'should work when union has no `id` field to select',
...WITH_SCHEMA,
code: /* GraphQL */ `
{
noIdOrNoId2 {
... on NoId {
name
}
... on NoId2 {
name
}
}
}
`,
},
],
invalid: [
{
Expand Down Expand Up @@ -429,5 +452,53 @@ ruleTester.run<RuleOptions, true>('require-selections', rule, {
},
},
},
{
name: 'should report an error with union and non-inline fragment',
errors: [MESSAGE_ID],
code: /* GraphQL */ `
{
userOrPost {
...UnionFragment
}
}
`,
parserOptions: {
graphQLConfig: {
schema: USER_POST_SCHEMA,
documents: /* GraphQL */ `
fragment UnionFragment on UserOrPost {
... on User {
name
}
}
`,
},
},
},
{
name: 'should report an error with union and non-inline fragment and nested fragment',
errors: [MESSAGE_ID],
code: /* GraphQL */ `
{
userOrPost {
...UnionFragment
}
}
`,
parserOptions: {
graphQLConfig: {
schema: USER_POST_SCHEMA,
documents: /* GraphQL */ `
fragment UnionFragment on UserOrPost {
...UserFields
}
fragment UserFields on User {
name
}
`,
},
},
},
],
});
15 changes: 14 additions & 1 deletion packages/plugin/src/rules/require-selections/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,25 @@ export const rule: GraphQLESLintRule<RuleOptions, true> = {
checkFields(rawType);
} else if (rawType instanceof GraphQLUnionType) {
for (const selection of node.selections) {
const types = rawType.getTypes();
if (selection.kind === Kind.INLINE_FRAGMENT) {
const types = rawType.getTypes();
const t = types.find(t => t.name === selection.typeCondition!.name.value);
if (t) {
checkFields(t);
}
} else if (selection.kind === Kind.FRAGMENT_SPREAD) {
const [foundSpread] = siblings.getFragment(selection.name.value);
if (!foundSpread) return;
const fragmentSpread = foundSpread.document;

// the fragment is either for the union type itself or one of the types in the union
const t =
fragmentSpread.typeCondition.name.value === rawType.name
? rawType
: types.find(t => t.name === fragmentSpread.typeCondition.name.value)!;
checkedFragmentSpreads.add(fragmentSpread.name.value);

checkSelections(fragmentSpread.selectionSet, t, loc, parent, checkedFragmentSpreads);
}
}
}
Expand Down
36 changes: 36 additions & 0 deletions packages/plugin/src/rules/require-selections/snapshot.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,42 @@ exports[`require-selections > invalid > should report an error with union and fr
7 | }
`;

exports[`require-selections > invalid > should report an error with union and non-inline fragment 1`] = `
#### ⌨️ Code

1 | {
2 | userOrPost {
3 | ...UnionFragment
4 | }
5 | }

#### ❌ Error

1 | {
> 2 | userOrPost {
| ^ Field \`userOrPost.id\` must be selected when it's available on a type.
Include it in your selection set or add to used fragment \`UnionFragment\`.
3 | ...UnionFragment
`;

exports[`require-selections > invalid > should report an error with union and non-inline fragment and nested fragment 1`] = `
#### ⌨️ Code

1 | {
2 | userOrPost {
3 | ...UnionFragment
4 | }
5 | }

#### ❌ Error

1 | {
> 2 | userOrPost {
| ^ Field \`userOrPost.id\` must be selected when it's available on a type.
Include it in your selection set or add to used fragments \`UnionFragment\` or \`UserFields\`.
3 | ...UnionFragment
`;

exports[`require-selections > invalid > support multiple id field names 1`] = `
#### ⌨️ Code

Expand Down

0 comments on commit b3c73dc

Please sign in to comment.