[v4] set GraphQL-Entity props required

[sebastian-ludwig]

Bug report

In your GraphQL-Example is this example:

type DocumentEntity {
  id: ID
  attributes: Document
}
​
type DocumentEntityResponse {
  data: DocumentEntity
}
​
type DocumentEntityResponseCollection {
  data: [DocumentEntity!]!
  meta: ResponseCollectionMeta!
}

ID and attributes shouldn't be optional in documentEntity, if there is a valid DocumentEntity data. Background: I create TypeScript-Classes for the Frontend based on the schema. In v3 I could just check if the data has arrived and then work with the data, in v4 I would need to check the data and then if id and attributes exists, that does not seem to be necessary.

Expected behavior

type DocumentEntity {
  id: ID!
  attributes: Document!
}

System

• Node.js version: v14.18.0
• NPM version: 6.14.15
• Strapi version: 4.0.0
• Database: PostgreSQL
• Operating system: MacOS

Solution

// @strapi/plugin-graphql/server/services/builders/entity.js
t.nonNull.id('id', { resolve: prop('id') });
t.field('attributes', {
  type: nonNull(typeName),
  resolve: identity,
});

[derrickmehaffy]

Hello,

I see you are wanting to ask a question that is not really a bug report or a feature request, questions should be directed to our forum. Please see the following contributing guidelines for asking a question here.

[sebastian-ludwig]

@derrickmehaffy Thank you, I should not have formulated it as a question. After taking another look into the Code, I consider this as a bug, so I updated my description, hope this is ok? Could you reopen the issue please?

[derrickmehaffy]

@derrickmehaffy Thank you, I should not have formulated it as a question. After taking another look into the Code, I consider this as a bug, so I updated my description, hope this is ok? Could you reopen the issue please?

You are missing the system block of the template, if you could please add that and respond back and I can reopen.

[sebastian-ludwig]

@derrickmehaffy Here we go

[christiaan-lombard]

This issue makes for a very bad experience when building a TS client application. Types generated from Strapi's GraphQL schema often contains null and undefined that requires mapping on the client side.

eg. Dynamic Zone

type Page {
  title: String!
  body: [PageBodyDynamicZone]!
}

maps to

export type Page = { 
  __typename: 'Page', 
  title: string,
  body: Array<PageBodyDynamicZone | null | undefined> // ?
}

eg. Query

query Pages {
  pages {
    data {
      id
      attributes {
        ...PageAttributes
      }
    }
  }
}

maps to

export type PagesQuery = {
  __typename: 'Query',
  pages?: null | undefined | { // ?
    __typename: 'PageEntityResponseCollection',
    data: Array<{
      __typename: 'PageEntity',
      id?: null | undefined | string, // ?
      attributes?: null | undefined | PageAttributes // ?
    }>
  }
};

Related issue: 9411

These types require copious amounts of useless null/undefined checks on the client side, not to mention mapping returned arrays with:

export function mapDefiniteArray<T>(source: Array<T | undefined | null>): Array<T> {
  return source as Array<T>;
}

Why are these GQL type definitions not required?

[dottodot]

Yes still definitely isn't right, the ID shouldn't ever be null or undefined.

[frankadrian]

Hello, I just experienced the same problem, type generated for Typescript contains null even though the ID for example should never be nullable. When setting a custom field to required via the Strapi admin the field also shows up as not nullable in the generated types. So this works as expected, but IDs should never be nullable. Please correct me if I'm wrong.

Here is an example of a Collection type using the graphql api extension for autogenerated endpoints:

export interface Parts_parts_data {
  __typename: "PartEntity";
  id: string | null; // <- id should not be nullable
  attributes: Parts_parts_data_attributes | null; // <- also here attributes should at least be an object of the said type
}

export interface Parts_parts {
  __typename: "PartEntityResponseCollection";
  data: Parts_parts_data[];
}

export interface Parts {
  parts: Parts_parts | null; // <- also should not be nullable. 
}

I would love to create an MR for this, is it something you would consider?

Kind regards

[itzaks]

Hi, I'm experiencing the same. I really enjoy strapi, but these properties being marked as nullable makes my typescript experience quite messy. Would love to see a fix on this.

[axelinternet]

Anyone have a workaround for this? Working with generated types from the graphql is such a mess.

Have you made workarounds in your scripts (to not follow the schema...) or something else?

[cesar3030]

Struggling with the same issue here. Even __typename is optional for me which is impossible.

[ehsmsh]

It seems to be a headache working with TS client like this. Any updates or workarounds?

[axelinternet]

How are you guys handing this in your applications? Huge headache for me

[rikkit]

Lots and lots of of ?....

[halavasty]

I had to abandon Strapi because of this behavior 🥹 so sorry

[christiaan-lombard]

We use mapping functions to get rid of these crappy types.

Here is a few examples:

export function mapEntity<A>(data: { id?: string | null; attributes?: A | null }): {
  id: string;
  attributes: A;
} {
  return data as { id: string; attributes: A };
}

export function mapDefiniteArray<T>(source: Array<T | undefined | null>): Array<T> {
  return source as Array<T>;
}

😒

[michalhonc]

We are patching the documentation plugin to make ID and attributes required

Patch file location (mind the lib version): patches/@strapi+plugin-documentation+4.15.2.patch Patch file content:

diff --git a/node_modules/@strapi/plugin-documentation/server/services/helpers/build-component-schema.js b/node_modules/@strapi/plugin-documentation/server/services/helpers/build-component-schema.js
 index 95fb824..54486c1 100644
 --- a/node_modules/@strapi/plugin-documentation/server/services/helpers/build-component-schema.js
 +++ b/node_modules/@strapi/plugin-documentation/server/services/helpers/build-component-schema.js
 @@ -222,6 +222,7 @@ const getAllSchemasForContentType = ({ routeInfo, attributes, uniqueName }) => {
        }),
      },
      [`${pascalCase(uniqueName)}ResponseDataObject`]: {
 +      required: ['id', 'attributes'],
        type: 'object',
        properties: {
          id: { type: 'number' },

and run (setup patch-package first)

npx patch-package @strapi/plugin-documentation

this will then generate those fields as required (tested for OpenAPI spec). Not a bulletproof solution so be cautious

[smonkey72]

I'm amazed that the ticket hasn't budged in 2 years. Working with Strapi/GraphQL in TypeScript projects feels almost impossible. 🤔

[momesana]

I am also struggling with this. Any feedback from the Strapi developers?

[momesana]

This is an immensely annoying bug. It's weird that it has been assigned a severity of 'low'. Lots of people use typescript these days and this bug makes it next to impossible to use typescript with the generated graphql types unless one litters the codebase with useless type assertions which are only there to make up for a bug that should've been addressed by the strapi team 2 years ago when the issue was first reported. Also none of the workarounds for graphql work anymore as the code has been refactored and shifted around beyond recognition since they were first described.

[momesana]

Any chance one of the Strapi maintainers (@derrickmehaffy maybe) may look at the PR put forward by @msadegh76 and approve it or give suggestions on how to improve it? The diff looks simple enough that a review could probably be done without too much effort.

[LufyCZ]

Here's a patch pulled from the PR:

patches/@strapi__plugin-graphql@4.22.1.patch

diff --git a/dist/server/index.js b/dist/server/index.js
index 5221af3c4a7c9bbb4aa3f201aa98b46fff29a5e8..04631eeb2de923fb121e76a3ba3898e1f90bdea1 100644
--- a/dist/server/index.js
+++ b/dist/server/index.js
@@ -1433,9 +1433,9 @@ const entity = ({ strapi: strapi2 }) => {
       return nexus.objectType({
         name,
         definition(t) {
-          t.id("id", { resolve: fp.prop("id") });
+          t.nonNull.id("id", { resolve: fp.prop("id") });
           if (!fp.isEmpty(attributes2)) {
-            t.field("attributes", {
+            t.nonNull.field("attributes", {
               type: typeName,
               resolve: fp.identity
             });
@@ -1780,7 +1780,7 @@ const createCollectionTypeQueriesBuilder = ({ strapi: strapi2 }) => {
     const { uid } = contentType2;
     const findQueryName = getFindQueryName(contentType2);
     const responseCollectionTypeName = getEntityResponseCollectionName(contentType2);
-    t.field(findQueryName, {
+    t.nonNull.field(findQueryName, {
       type: responseCollectionTypeName,
       args: getContentTypeArgs(contentType2),
       async resolve(parent, args2, ctx) {
diff --git a/dist/server/index.mjs b/dist/server/index.mjs
index 6fba041a442521c3e645a5034cc65fe785eb4ecf..a8c488e60924851160a90793e14bcc0be447b101 100644
--- a/dist/server/index.mjs
+++ b/dist/server/index.mjs
@@ -1412,9 +1412,9 @@ const entity = ({ strapi: strapi2 }) => {
       return objectType({
         name,
         definition(t) {
-          t.id("id", { resolve: prop("id") });
+          t.nonNull.id("id", { resolve: prop("id") });
           if (!isEmpty(attributes2)) {
-            t.field("attributes", {
+            t.nonNull.field("attributes", {
               type: typeName,
               resolve: identity
             });
@@ -1759,7 +1759,7 @@ const createCollectionTypeQueriesBuilder = ({ strapi: strapi2 }) => {
     const { uid } = contentType2;
     const findQueryName = getFindQueryName(contentType2);
     const responseCollectionTypeName = getEntityResponseCollectionName(contentType2);
-    t.field(findQueryName, {
+    t.nonNull.field(findQueryName, {
       type: responseCollectionTypeName,
       args: getContentTypeArgs(contentType2),
       async resolve(parent, args2, ctx) {

(btw it's crazy that this is somehow still open)

[Maximaximum]

Wow, this bug is EXTREMELY annoying, it's a shame the Strapi team has not paid any attention to resolving this

[momesana]

Wow, this bug is EXTREMELY annoying, it's a shame the Strapi team has not paid any attention to resolving this

Either they are working on a new release that does potentially contain a fix and that's why they don't merge this, or the (inappropriately) low priority assigned to this issue is the reason for it being ignored. In the first case, it should nevertheless be clearly communicated by the team that the issue will be addressed by the next major release and in the later case the Strapi team should IMHO reconsider the current priority assigned to it.

[derrickmehaffy]

I'll bump this to medium, I honestly am not sure if it's already been patched in the Strapi 5 RC releases or not as we rewrote a pretty large part of the GraphQL plugin and I personally don't like/don't touch typescript.

[Aurelsicoko]

@derrickmehaffy I don't know either; I'm checking with the team to see how we could improve this quickly. Hopefully, the new API format and improvements on Strapi 5 will help.

[alexandrebodin]

Hey, In v5 attributes are removed in favor of a flat strucutre (we do provide a compatibility mode to ease the migration). In both case they will be required.

[axelinternet]

@alexandrebodin will the attributes reflect the required or nullable status in Strapi? Having a stable API that is properly typed is the main thing I look at when picking solutions for my clients. The editing functionality and features are pretty much the same across the different CMS / low-code offerings but these are the things that cost so much in terms of development time.