Cms: FR: Stricter required field types for GraphQL schema

[OscarBarrett]

Currently the generated GraphQL schema treats all fields as nullable, even if they are marked as required for a given Entry type. When generating types from the schema using introspection this results in many existence checks that should otherwise be avoidable.

For example: You have a Section with a required Entries field for an Section that has a required Plain Text field. You are using TypeScript to consume the GraphQL API and want to get an array of the text fields as strings:

{
  entries(section: "mySection") {
    ...on mySection_mySection_Entry {
      anotherSection {
        ...on anotherSection_anotherSection_Entry {
          textField
        }
      }
    }
  }
}

// Example generated types from introspection
type Maybe<T> = T | null;

Type MySection = ElementInterface & EntryInterface & {
  ...
  anotherSection?: Maybe<Array<Maybe<EntryInterface>>>;
}

Type AnotherSection = ElementInterface & EntryInterface & {
  ...
  textField?: Maybe<Scalars['String']>;
}

const allText = data.mySection?.map(entry => entry?.anotherSection?.[0]?.textField)
  .filter((text): text is string => text !== null && text !== undefined);

This can become quite a headache if you're consuming a lot of data from GraphQL that you know should exist, and can get especially verbose with deeply nested fields.

If the GraphQL schema instead marked required fields as non-nullable, then the checks could be much less verbose. For example:

// Ideal generated types
Type MySection = ElementInterface & EntryInterface & {
  ...
  anotherSection: Array<EntryInterface>;
}

Type AnotherSection = ElementInterface & EntryInterface & {
  ...
  textField: Scalars['String'];
}

const allText = data.mySection?.map(entry => entry.anotherSection[0]?.textField);

In addition, the Interfaces have nullable fields when potentially they should always be non-null. For example, is it possible for a type implementing AssetInterface to have a url of null? Or an id of null?

Currently validation is not applied retroactively, so if a field on a Section is changed from optional to required then entries may be returned with null values for the field if they previously had no value for this newly required field. If stricter types were to be implemented it would then probably need to:

• Sit behind a config flag, disabled by default for backwards compatibility, and
• Ensure only valid entries are returned when enabled.

[andris-sevcenko]

Ah, good catch! I added the correct nullable behavior for mutations, but this slipped my mind.

[andris-sevcenko]

This has been fixed for 3.5.

[FreekVR]

Hey @andris-sevcenko ,

I was just checking out the HEAD version of the 3.5 branch but I feel like this change introduced some errors upon running introspection queries, where it's complaining about a ton of arguments in the schema not being a nullable type.

Shortened version:

"Expected {\"name\":\"linkEntry\",\"type\":\"[EntryInterface]\",\"args\":{\"id\":{\"name\":\"id\",\"type\":\"[QueryArgument]\",\"description\":\"Narrows the query results based on the elements\\u2019 IDs.\"},\" [...]},\"resolve\":\"craft\\\\gql\\\\resolvers\\\\elements\\\\Entry::resolve\"} to be a GraphQL nullable type."

The full error is actually:

Expected {\"name\":\"linkEntry\",\"type\":\"[EntryInterface]\",\"args\":{\"id\":{\"name\":\"id\",\"type\":\"[QueryArgument]\",\"description\":\"Narrows the query results based on the elements\\u2019 IDs.\"},\"uid\":{\"name\":\"uid\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on the elements\\u2019 UIDs.\"},\"drafts\":{\"name\":\"drafts\",\"type\":\"Boolean\",\"description\":\"Whether draft elements should be returned.\"},\"draftOf\":{\"name\":\"draftOf\",\"type\":\"QueryArgument\",\"description\":\"The source element ID that drafts should be returned for. Set to `false` to fetch unsaved drafts.\"},\"draftId\":{\"name\":\"draftId\",\"type\":\"Int\",\"description\":\"The ID of the draft to return (from the `drafts` table)\"},\"draftCreator\":{\"name\":\"draftCreator\",\"type\":\"Int\",\"description\":\"The drafts\\u2019 creator ID\"},\"revisions\":{\"name\":\"revisions\",\"type\":\"Boolean\",\"description\":\"Whether revision elements should be returned.\"},\"revisionOf\":{\"name\":\"revisionOf\",\"type\":\"QueryArgument\",\"description\":\"The source element ID that revisions should be returned for\"},\"revisionId\":{\"name\":\"revisionId\",\"type\":\"Int\",\"description\":\"The ID of the revision to return (from the `revisions` table)\"},\"revisionCreator\":{\"name\":\"revisionCreator\",\"type\":\"Int\",\"description\":\"The revisions\\u2019 creator ID\"},\"status\":{\"name\":\"status\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on the elements\\u2019 statuses.\"},\"archived\":{\"name\":\"archived\",\"type\":\"Boolean\",\"description\":\"Narrows the query results to only elements that have been archived.\"},\"trashed\":{\"name\":\"trashed\",\"type\":\"Boolean\",\"description\":\"Narrows the query results to only elements that have been soft-deleted.\"},\"site\":{\"name\":\"site\",\"type\":\"[String]\",\"description\":\"Determines which site(s) the elements should be queried in. Defaults to the current (requested) site.\"},\"siteId\":{\"name\":\"siteId\",\"type\":\"String\",\"description\":\"Determines which site(s) the elements should be queried in. Defaults to the current (requested) site.\"},\"unique\":{\"name\":\"unique\",\"type\":\"Boolean\",\"description\":\"Determines whether only elements with unique IDs should be returned by the query.\"},\"enabledForSite\":{\"name\":\"enabledForSite\",\"type\":\"Boolean\",\"description\":\"Narrows the query results based on whether the elements are enabled in the site they\\u2019re being queried in, per the `site` argument.\"},\"title\":{\"name\":\"title\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on the elements\\u2019 titles.\"},\"slug\":{\"name\":\"slug\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on the elements\\u2019 slugs.\"},\"uri\":{\"name\":\"uri\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on the elements\\u2019 URIs.\"},\"search\":{\"name\":\"search\",\"type\":\"String\",\"description\":\"Narrows the query results to only elements that match a search query.\"},\"relatedTo\":{\"name\":\"relatedTo\",\"type\":\"[Int]\",\"description\":\"Narrows the query results to elements that relate to *any* of the provided element IDs. This argument is ignored, if `relatedToAll` is also used.\"},\"relatedToAll\":{\"name\":\"relatedToAll\",\"type\":\"[Int]\",\"description\":\"Narrows the query results to elements that relate to *all* of the provided element IDs. Using this argument will cause `relatedTo` argument to be ignored.\"},\"ref\":{\"name\":\"ref\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on a reference string.\"},\"fixedOrder\":{\"name\":\"fixedOrder\",\"type\":\"Boolean\",\"description\":\"Causes the query results to be returned in the order specified by the `id` argument.\"},\"inReverse\":{\"name\":\"inReverse\",\"type\":\"Boolean\",\"description\":\"Causes the query results to be returned in reverse order.\"},\"dateCreated\":{\"name\":\"dateCreated\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on the elements\\u2019 creation dates.\"},\"dateUpdated\":{\"name\":\"dateUpdated\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on the elements\\u2019 last-updated dates.\"},\"offset\":{\"name\":\"offset\",\"type\":\"Int\",\"description\":\"Sets the offset for paginated results.\"},\"limit\":{\"name\":\"limit\",\"type\":\"Int\",\"description\":\"Sets the limit for paginated results.\"},\"orderBy\":{\"name\":\"orderBy\",\"type\":\"String\",\"description\":\"Sets the field the returned elements should be ordered by\"},\"withStructure\":{\"name\":\"withStructure\",\"type\":\"Boolean\",\"description\":\"Explicitly determines whether the query should join in the structure data.\"},\"structureId\":{\"name\":\"structureId\",\"type\":\"Int\",\"description\":\"Determines which structure data should be joined into the query.\"},\"level\":{\"name\":\"level\",\"type\":\"Int\",\"description\":\"Narrows the query results based on the elements\\u2019 level within the structure.\"},\"hasDescendants\":{\"name\":\"hasDescendants\",\"type\":\"Boolean\",\"description\":\"Narrows the query results based on whether the elements have any descendants.\"},\"ancestorOf\":{\"name\":\"ancestorOf\",\"type\":\"Int\",\"description\":\"Narrows the query results to only elements that are ancestors of another element.\"},\"ancestorDist\":{\"name\":\"ancestorDist\",\"type\":\"Int\",\"description\":\"Narrows the query results to only elements that are up to a certain distance away from the element specified by `ancestorOf`.\"},\"descendantOf\":{\"name\":\"descendantOf\",\"type\":\"Int\",\"description\":\"Narrows the query results to only elements that are descendants of another element.\"},\"descendantDist\":{\"name\":\"descendantDist\",\"type\":\"Int\",\"description\":\"Narrows the query results to only elements that are up to a certain distance away from the element specified by `descendantOf`.\"},\"leaves\":{\"name\":\"leaves\",\"type\":\"Boolean\",\"description\":\"Narrows the query results based on whether the elements are \\u201cleaves\\u201d (element with no descendants).\"},\"nextSiblingOf\":{\"name\":\"nextSiblingOf\",\"type\":\"Int\",\"description\":\"Narrows the query results to only the entry that comes immediately after another element.\"},\"prevSiblingOf\":{\"name\":\"prevSiblingOf\",\"type\":\"Int\",\"description\":\"Narrows the query results to only the entry that comes immediately before another element.\"},\"positionedAfter\":{\"name\":\"positionedAfter\",\"type\":\"Int\",\"description\":\"Narrows the query results to only entries that are positioned after another element.\"},\"positionedBefore\":{\"name\":\"positionedBefore\",\"type\":\"Int\",\"description\":\"Narrows the query results to only entries that are positioned before another element.\"},\"featuredImage\":{\"name\":\"featuredImage\",\"type\":\"[QueryArgument]\"},\"shortDescription\":{\"name\":\"shortDescription\",\"type\":\"[QueryArgument]\"},\"seo\":{\"name\":\"seo\",\"type\":\"[QueryArgument]\"},\"topic\":{\"name\":\"topic\",\"type\":\"[QueryArgument]\"},\"isFeatured\":{\"name\":\"isFeatured\",\"type\":\"Boolean\"},\"tags\":{\"name\":\"tags\",\"type\":\"[QueryArgument]\"},\"subTitle\":{\"name\":\"subTitle\",\"type\":\"[QueryArgument]\"},\"tagline\":{\"name\":\"tagline\",\"type\":\"[QueryArgument]\"},\"startDate\":{\"name\":\"startDate\",\"type\":\"[QueryArgument]\"},\"location\":{\"name\":\"location\",\"type\":\"[QueryArgument]\"},\"costs\":{\"name\":\"costs\",\"type\":\"[QueryArgument]\"},\"organization\":{\"name\":\"organization\",\"type\":\"[QueryArgument]\"},\"editable\":{\"name\":\"editable\",\"type\":\"Boolean\",\"description\":\"Whether to only return entries that the user has permission to edit.\"},\"section\":{\"name\":\"section\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on the section handles the entries belong to.\"},\"sectionId\":{\"name\":\"sectionId\",\"type\":\"[QueryArgument]\",\"description\":\"Narrows the query results based on the sections the entries belong to, per the sections\\u2019 IDs.\"},\"type\":{\"name\":\"type\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on the entries\\u2019 entry type handles.\"},\"typeId\":{\"name\":\"typeId\",\"type\":\"[QueryArgument]\",\"description\":\"Narrows the query results based on the entries\\u2019 entry types, per the types\\u2019 IDs.\"},\"authorId\":{\"name\":\"authorId\",\"type\":\"[QueryArgument]\",\"description\":\"Narrows the query results based on the entries\\u2019 authors.\"},\"authorGroup\":{\"name\":\"authorGroup\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on the user group the entries\\u2019 authors belong to.\"},\"authorGroupId\":{\"name\":\"authorGroupId\",\"type\":\"[QueryArgument]\",\"description\":\"Narrows the query results based on the user group the entries\\u2019 authors belong to, per the groups\\u2019 IDs.\"},\"postDate\":{\"name\":\"postDate\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on the entries\\u2019 post dates.\"},\"before\":{\"name\":\"before\",\"type\":\"String\",\"description\":\"Narrows the query results to only entries that were posted before a certain date.\"},\"after\":{\"name\":\"after\",\"type\":\"String\",\"description\":\"Narrows the query results to only entries that were posted on or after a certain date.\"},\"expiryDate\":{\"name\":\"expiryDate\",\"type\":\"[String]\",\"description\":\"Narrows the query results based on the entries\\u2019 expiry dates.\"}},\"resolve\":\"craft\\\\gql\\\\resolvers\\\\elements\\\\Entry::resolve\"} to be a GraphQL nullable type."

[andris-sevcenko]

Ugh. Okay. This needs to be a little more involved change, it seems.

This is now caused by the same field being required in some scenarios, but not required in other scenarios. So, Craft is re-using the same GQL type, when it can be a different GQL type.

I'll fix this in a jiffy!

[andris-sevcenko]

Well, maybe not in a short jiffy. This exposed an underlying bug-slash-oversight.

[andris-sevcenko]

@FreekVR aight, fixed it for good. Hopefully :)

[FreekVR]

Awesome, thanks! We're not running an earlier commit hash that had the GQL features we need but I'll check it out again soonish ;)

[andris-sevcenko]

Heads up, @OscarBarrett, this fix will be reverted and Craft won't be marking fields as required in GraphQL schema, even if they are in Craft.

This is because there are multiple scenarios where a required field would not have a value, for example, when saving a draft.

[andris-sevcenko]

Alright. Closing this and marking as "wontfix", since implementing this directly blocks querying for drafts, automatically created singles and globals, and all elements that have had a required field added to them since their last save.