[relay-page-info]: allow nullable startCursor and endCursor if th…

…ere are no results (#1010) * [relay-page-info]: allow nullable `startCursor` and `endCursor` if there are no results * update docs * simplify
dimaMachina · Mar 31, 2022 · cf687a5 · cf687a5
1 parent 24762d9
commit cf687a5
Show file tree

Hide file tree

Showing 13 changed files with 76 additions and 69 deletions.
diff --git a/.changeset/stale-rice-pay.md b/.changeset/stale-rice-pay.md
@@ -0,0 +1,5 @@
+---
+'@graphql-eslint/eslint-plugin': patch
+---
+
+[relay-page-info]: allow nullable `startCursor` and `endCursor` if there are no results
diff --git a/.github/labels.yml b/.github/labels.yml
@@ -1,9 +1,3 @@
-- color: d73a4a
-  description: Something isn't working
-  name: bug
-- color: e5c420
-  description: ''
-  name: core
 - color: 0366d6
   description: Pull requests that update a dependency file
   name: dependencies
@@ -13,27 +7,22 @@
 - color: cfd3d7
   description: This issue or pull request already exists
   name: duplicate
-- color: a2eeef
-  description: New feature or request
-  name: enhancement
 - color: 7057ff
   description: Good for newcomers
   name: good first issue
-- color: '008672'
+- color: 008672
   description: Extra attention is needed
   name: help wanted
 - color: e4e669
   description: This doesn't seem right
   name: invalid
 - color: 50e087
-  description: ''
-  name: new-rule
+  name: new rule
 - color: d876e3
   description: Further information is requested
   name: question
 - color: f78ff0
-  description: ''
-  name: waiting-for-release
+  name: waiting for release
 - color: ffffff
   description: This will not be worked on
   name: wontfix
diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml
@@ -60,7 +60,7 @@ jobs:
 
       - name: Release Canary
         id: canary
-        uses: 'kamilkisiela/release-canary@master'
+        uses: kamilkisiela/release-canary@master
         if: github.repository == 'B2o5T/graphql-eslint'
         with:
           npm-token: ${{secrets.NODE_AUTH_TOKEN}}
@@ -69,7 +69,7 @@ jobs:
 
       - name: Publish a message
         if: steps.canary.outputs.released
-        uses: 'kamilkisiela/pr-comment@master'
+        uses: kamilkisiela/pr-comment@master
         with:
           message: |
             The latest changes of this PR are available as alpha in npm (based on the declared `changesets`):
@@ -82,7 +82,7 @@ jobs:
 
       - name: Publish an empty message
         if: steps.canary.outputs.released == 'false'
-        uses: 'kamilkisiela/pr-comment@master'
+        uses: kamilkisiela/pr-comment@master
         with:
           message: The latest changes of this PR are not available as alpha, since there are no linked `changesets` for this PR.
           bot-token: ${{secrets.GH_API_TOKEN}}

diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -50,7 +50,7 @@ jobs:
 
       - name: Create Release Pull Request or Publish to npm
         id: changesets
-        uses: changesets/action@master
+        uses: changesets/action@v1
         with:
           publish: yarn release
           commit: 'chore(release): update monorepo packages versions'

diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -17,7 +17,7 @@ jobs:
           fetch-depth: 0
 
       - name: Use Node
-        uses: actions/setup-node@master
+        uses: actions/setup-node@v3
         with:
           node-version: 16
 
@@ -50,7 +50,7 @@ jobs:
           fetch-depth: 0
 
       - name: Use Node
-        uses: actions/setup-node@master
+        uses: actions/setup-node@v3
         with:
           node-version: 16
 
@@ -73,7 +73,7 @@ jobs:
         run: yarn build
 
       - name: Upload build artifact
-        uses: actions/upload-artifact@master
+        uses: actions/upload-artifact@v3
         with:
           name: build-artifact
           path: packages/plugin/dist
@@ -96,7 +96,7 @@ jobs:
           fetch-depth: 0
 
       - name: Use Node ${{matrix.node_version}}
-        uses: actions/setup-node@master
+        uses: actions/setup-node@v3
         with:
           node-version: ${{matrix.node_version}}
 
@@ -130,7 +130,7 @@ jobs:
       # We need build for examples.spec.ts test
       # Otherwise we'll get error - Cannot find module 'node_modules/@graphql-eslint/eslint-plugin/dist/index.js'
       - name: Download build artifact
-        uses: actions/download-artifact@master
+        uses: actions/download-artifact@v3
         with:
           name: build-artifact
           path: packages/plugin/dist

diff --git a/docs/rules/relay-connection-types.md b/docs/rules/relay-connection-types.md
@@ -9,7 +9,7 @@ Set of rules to follow Relay specification for Connection types.
 
 - Any type whose name ends in "Connection" is considered by spec to be a `Connection type`
 - Connection type must be an Object type
-- Connection type must contain a field `edges` that return a list type which wraps an edge type
+- Connection type must contain a field `edges` that return a list type that wraps an edge type
 - Connection type must contain a field `pageInfo` that return a non-null `PageInfo` Object type
 
 ## Usage Examples

diff --git a/docs/rules/relay-edge-types.md b/docs/rules/relay-edge-types.md
@@ -9,8 +9,8 @@ Set of rules to follow Relay specification for Edge types.
 
 - A type that is returned in list form by a connection type's `edges` field is considered by this spec to be an Edge type
 - Edge type must be an Object type
-- Edge type must contain a field `node` that return either a Scalar, Enum, Object, Interface, Union, or a non-null wrapper around one of those types. Notably, this field cannot return a list
-- Edge type must contain a field `cursor` that return a String, Scalar, or a non-null wrapper wrapper around one of those types
+- Edge type must contain a field `node` that return either Scalar, Enum, Object, Interface, Union, or a non-null wrapper around one of those types. Notably, this field cannot return a list
+- Edge type must contain a field `cursor` that return either String, Scalar, or a non-null wrapper around one of those types
 - Edge type name must end in "Edge" _(optional)_
 - Edge type's field `node` must implement `Node` interface _(optional)_
 - A list type should only wrap an edge type _(optional)_

diff --git a/docs/rules/relay-page-info.md b/docs/rules/relay-page-info.md
@@ -8,8 +8,8 @@
 Set of rules to follow Relay specification for `PageInfo` object.
 
 - `PageInfo` must be an Object type
-- `PageInfo` must contain fields `hasPreviousPage` and `hasNextPage`, which return non-null Boolean
-- `PageInfo` must contain fields `startCursor` and `endCursor`, which return non-null String or Scalar
+- `PageInfo` must contain fields `hasPreviousPage` and `hasNextPage`, that return non-null Boolean
+- `PageInfo` must contain fields `startCursor` and `endCursor`, that return either String, Scalar, or a non-null wrapper around one of those types
 
 ## Usage Examples
 

diff --git a/packages/plugin/src/rules/relay-connection-types.ts b/packages/plugin/src/rules/relay-connection-types.ts
@@ -1,6 +1,6 @@
 import { Kind, ObjectTypeDefinitionNode, TypeNode } from 'graphql';
-import { GraphQLESLintRule } from '../types';
-import { GraphQLESTreeNode } from '../estree-converter';
+import type { GraphQLESLintRule } from '../types';
+import type { GraphQLESTreeNode } from '../estree-converter';
 
 const MUST_BE_OBJECT_TYPE = 'MUST_BE_OBJECT_TYPE';
 const MUST_CONTAIN_FIELD_EDGES = 'MUST_CONTAIN_FIELD_EDGES';
@@ -38,7 +38,7 @@ const rule: GraphQLESLintRule = {
         '',
         '- Any type whose name ends in "Connection" is considered by spec to be a `Connection type`',
         '- Connection type must be an Object type',
-        '- Connection type must contain a field `edges` that return a list type which wraps an edge type',
+        '- Connection type must contain a field `edges` that return a list type that wraps an edge type',
         '- Connection type must contain a field `pageInfo` that return a non-null `PageInfo` Object type',
       ].join('\n'),
       url: 'https://github.com/B2o5T/graphql-eslint/blob/master/docs/rules/relay-connection-types.md',

diff --git a/packages/plugin/src/rules/relay-edge-types.ts b/packages/plugin/src/rules/relay-edge-types.ts
@@ -11,9 +11,9 @@ import {
 } from 'graphql';
 import { getDocumentNodeFromSchema } from '@graphql-tools/utils';
 import { getTypeName, requireGraphQLSchemaFromContext } from '../utils';
-import { GraphQLESLintRule } from '../types';
-import { GraphQLESTreeNode } from '../estree-converter';
-import { GraphQLESLintRuleListener } from '../testkit';
+import type { GraphQLESLintRule } from '../types';
+import type { GraphQLESTreeNode } from '../estree-converter';
+import type { GraphQLESLintRuleListener } from '../testkit';
 
 const RULE_ID = 'relay-edge-types';
 const MESSAGE_MUST_BE_OBJECT_TYPE = 'MESSAGE_MUST_BE_OBJECT_TYPE';
@@ -71,8 +71,8 @@ const rule: GraphQLESLintRule<[EdgeTypesConfig], true> = {
         '',
         "- A type that is returned in list form by a connection type's `edges` field is considered by this spec to be an Edge type",
         '- Edge type must be an Object type',
-        '- Edge type must contain a field `node` that return either a Scalar, Enum, Object, Interface, Union, or a non-null wrapper around one of those types. Notably, this field cannot return a list',
-        '- Edge type must contain a field `cursor` that return a String, Scalar, or a non-null wrapper wrapper around one of those types',
+        '- Edge type must contain a field `node` that return either Scalar, Enum, Object, Interface, Union, or a non-null wrapper around one of those types. Notably, this field cannot return a list',
+        '- Edge type must contain a field `cursor` that return either String, Scalar, or a non-null wrapper around one of those types',
         '- Edge type name must end in "Edge" _(optional)_',
         "- Edge type's field `node` must implement `Node` interface _(optional)_",
         '- A list type should only wrap an edge type _(optional)_',

diff --git a/packages/plugin/src/rules/relay-page-info.ts b/packages/plugin/src/rules/relay-page-info.ts
@@ -1,5 +1,5 @@
-import { GraphQLESLintRule } from '../types';
-import { GraphQLESTreeNode } from '../estree-converter';
+import type { GraphQLESLintRule } from '../types';
+import type { GraphQLESTreeNode } from '../estree-converter';
 import { isScalarType, Kind, ObjectTypeDefinitionNode } from 'graphql';
 import { NON_OBJECT_TYPES } from './relay-connection-types';
 import { REPORT_ON_FIRST_CHARACTER, requireGraphQLSchemaFromContext } from '../utils';
@@ -20,8 +20,8 @@ const rule: GraphQLESLintRule = {
         'Set of rules to follow Relay specification for `PageInfo` object.',
         '',
         '- `PageInfo` must be an Object type',
-        '- `PageInfo` must contain fields `hasPreviousPage` and `hasNextPage`, which return non-null Boolean',
-        '- `PageInfo` must contain fields `startCursor` and `endCursor`, which return non-null String or Scalar',
+        '- `PageInfo` must contain fields `hasPreviousPage` and `hasNextPage`, that return non-null Boolean',
+        '- `PageInfo` must contain fields `startCursor` and `endCursor`, that return either String, Scalar, or a non-null wrapper around one of those types',
       ].join('\n'),
       url: `https://github.com/B2o5T/graphql-eslint/blob/master/docs/rules/${RULE_ID}.md`,
       examples: [
@@ -70,22 +70,35 @@ const rule: GraphQLESLintRule = {
           typeName: 'Boolean' | 'String'
         ): void => {
           const field = fieldMap[fieldName];
-          const hasField = Boolean(field);
           const isStringType = typeName === 'String';
-          const isAllowedNonNullType =
-            hasField &&
-            field.gqlType.kind === Kind.NON_NULL_TYPE &&
-            field.gqlType.gqlType.kind === Kind.NAMED_TYPE &&
-            (field.gqlType.gqlType.name.value === typeName ||
-              (typeName === 'String' && isScalarType(schema.getType(field.gqlType.gqlType.name.value))));
 
-          if (!isAllowedNonNullType) {
-            const returnType = isStringType ? 'String or Scalar' : typeName;
+          let isAllowedType = false;
+          if (field) {
+            let type = field.gqlType;
+            if (typeName === 'Boolean') {
+              isAllowedType =
+                type.kind === Kind.NON_NULL_TYPE &&
+                type.gqlType.kind === Kind.NAMED_TYPE &&
+                type.gqlType.name.value === 'Boolean';
+            } else {
+              if (type.kind === Kind.NON_NULL_TYPE) {
+                type = type.gqlType;
+              }
+              if (type.kind === Kind.NAMED_TYPE) {
+                isAllowedType = type.name.value === 'String' || isScalarType(schema.getType(type.name.value));
+              }
+            }
+          }
+
+          if (!isAllowedType) {
+            const returnType = isStringType
+              ? 'either String, Scalar, or a non-null wrapper around one of those types'
+              : 'non-null Boolean';
             context.report({
-              node: hasField ? field.name : node.name,
-              message: hasField
-                ? `Field \`${fieldName}\` must return non-null ${returnType}.`
-                : `\`PageInfo\` must contain a field \`${fieldName}\`, that return non-null ${returnType}.`,
+              node: field ? field.name : node.name,
+              message: field
+                ? `Field \`${fieldName}\` must return ${returnType}.`
+                : `\`PageInfo\` must contain a field \`${fieldName}\`, that return ${returnType}.`,
             });
           }
         };

diff --git a/packages/plugin/tests/__snapshots__/relay-page-info.spec.md b/packages/plugin/tests/__snapshots__/relay-page-info.spec.md
@@ -73,48 +73,48 @@ exports[`when extend type 1`] = `
 #### ❌ Error 3/4
 
     > 1 |         type PageInfo
-        |              ^^^^^^^^ \`PageInfo\` must contain a field \`startCursor\`, that return non-null String or Scalar.
+        |              ^^^^^^^^ \`PageInfo\` must contain a field \`startCursor\`, that return either String, Scalar, or a non-null wrapper around one of those types.
       2 |         extend type PageInfo {
 
 #### ❌ Error 4/4
 
     > 1 |         type PageInfo
-        |              ^^^^^^^^ \`PageInfo\` must contain a field \`endCursor\`, that return non-null String or Scalar.
+        |              ^^^^^^^^ \`PageInfo\` must contain a field \`endCursor\`, that return either String, Scalar, or a non-null wrapper around one of those types.
       2 |         extend type PageInfo {
 `;
 
 exports[`when fields is missing or incorrect return type 1`] = `
 #### ⌨️ Code
 
       1 |         type PageInfo {
-      2 |           hasPreviousPage: Boolean
-      3 |           startCursor: String
+      2 |           hasPreviousPage: [Boolean!]!
+      3 |           startCursor: [String]
       4 |         }
 
 #### ❌ Error 1/4
 
     > 1 |         type PageInfo {
         |              ^^^^^^^^ \`PageInfo\` must contain a field \`hasNextPage\`, that return non-null Boolean.
-      2 |           hasPreviousPage: Boolean
+      2 |           hasPreviousPage: [Boolean!]!
 
 #### ❌ Error 2/4
 
     > 1 |         type PageInfo {
-        |              ^^^^^^^^ \`PageInfo\` must contain a field \`endCursor\`, that return non-null String or Scalar.
-      2 |           hasPreviousPage: Boolean
+        |              ^^^^^^^^ \`PageInfo\` must contain a field \`endCursor\`, that return either String, Scalar, or a non-null wrapper around one of those types.
+      2 |           hasPreviousPage: [Boolean!]!
 
 #### ❌ Error 3/4
 
       1 |         type PageInfo {
-    > 2 |           hasPreviousPage: Boolean
+    > 2 |           hasPreviousPage: [Boolean!]!
         |           ^^^^^^^^^^^^^^^ Field \`hasPreviousPage\` must return non-null Boolean.
-      3 |           startCursor: String
+      3 |           startCursor: [String]
 
 #### ❌ Error 4/4
 
-      2 |           hasPreviousPage: Boolean
-    > 3 |           startCursor: String
-        |           ^^^^^^^^^^^ Field \`startCursor\` must return non-null String or Scalar.
+      2 |           hasPreviousPage: [Boolean!]!
+    > 3 |           startCursor: [String]
+        |           ^^^^^^^^^^^ Field \`startCursor\` must return either String, Scalar, or a non-null wrapper around one of those types.
       4 |         }
 `;
 

diff --git a/packages/plugin/tests/relay-page-info.spec.ts b/packages/plugin/tests/relay-page-info.spec.ts
@@ -19,7 +19,7 @@ ruleTester.runGraphQLTests('relay-page-info', rule, {
         hasPreviousPage: Boolean!
         hasNextPage: Boolean!
         startCursor: String!
-        endCursor: String!
+        endCursor: String # can be null
       }
     `),
     {
@@ -28,7 +28,7 @@ ruleTester.runGraphQLTests('relay-page-info', rule, {
         type PageInfo {
           hasPreviousPage: Boolean!
           hasNextPage: Boolean!
-          startCursor: Int!
+          startCursor: Int # can be null
           endCursor: Float!
         }
       `),
@@ -110,8 +110,8 @@ ruleTester.runGraphQLTests('relay-page-info', rule, {
       name: 'when fields is missing or incorrect return type',
       ...useSchema(/* GraphQL */ `
         type PageInfo {
-          hasPreviousPage: Boolean
-          startCursor: String
+          hasPreviousPage: [Boolean!]!
+          startCursor: [String]
         }
       `),
       errors: 4,