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

GraphQL Schema is not respecting required fields #10610

Open
3 tasks done
roydigerhund opened this issue Dec 19, 2021 · 11 comments
Open
3 tasks done

GraphQL Schema is not respecting required fields #10610

roydigerhund opened this issue Dec 19, 2021 · 11 comments
Labels

Comments

@roydigerhund
Copy link

Preflight Checklist

Describe the Bug

The included GraphQL endpoint under /graphql does not seem to handle the required fields of collections.
All required fields should be marked as non-nullable in the schema but this is not the case.

This is a problem since tools like graphql-codegen will use this schema to generate the types for TypeScript. Required fields not marked as non-nullable will lead to wrong types and a worse experience for developers.

To Reproduce

  1. Create a collection with some required fields
  2. Query the schema of the collection
  3. The required fields are not marked as non-nullable

Errors Shown

No response

What version of Directus are you using?

9.3.0

What version of Node.js are you using?

16.13.0

What database are you using?

Postgres 13

What browser are you using?

Safari

What operating system are you using?

macOS

How are you deploying Directus?

running locally

@johannesfritsch
Copy link

I can confirm. My IDs are nullable too.

@axelinternet
Copy link

I'm also affected by this issue. Adds a need for a lot workaround that takes away a lot of the "magic" of working with graphql.

@kepi
Copy link
Contributor

kepi commented Sep 6, 2022

This simple change: main...kepi:directus:fix/graphqlid-nonnull make IDs non-nullable for me and my codebase is running ok. I'm just not sure if this is correct approach.

It will be great if anybody can provide feedback, as I really don't know directus codebase and just tried to find where to hack the correct part. I'll try to run tests when I'll have all test containers downloaded, so far not sure if I didn't break something else.

@rijkvanzanten
Copy link
Member

Linear: ENG-283

@thiras
Copy link

thiras commented Dec 26, 2022

Any progress toward global nullable/non-nullable on GraphQL?

@rijkvanzanten
Copy link
Member

Any progress toward global nullable/non-nullable on GraphQL?

Unfortunately, we can not provide estimates on features or fixes. As of now, the only way to get an accurate timeline or release date is to sponsor this task. We recognize that this may be an important feature/fix, but we are a small open-source organization with a lot to triage and complete. If you can't sponsor this ticket, then the next best thing is to increase its priority by giving it a 👍 . Or contribute a pull-request... after all, this is OSS!


This is an automated response.

@samthompsonkennedy
Copy link

This is actually incredibly frustrating, as our frontend builds the typings from GraphQL, so everything is possibly null...

@heho
Copy link

heho commented May 21, 2023

it is frustrating and for me close to a deal breaker. Here is what i found.

If you create a field as non nullable and give it no default value it is indeed non nullable.
If you create a field as non nullable and give it a default value it will be nullable in the schema.
I don't know if this is in relation with this code but it seems to be.

However it gets really strange and somewhat concerning. If one is to add a default value, the schema becomes nullable (this is to be expected). But this behaviour just stays. If one removes the default value, it will not become nullable again. however it sometimes seem to flip back if one adds and removes the nullability check box. It is really weird and does not seem stable. One has to hope this only affects the exported schema and does not change the actual database.

@evangeloszotos
Copy link

evangeloszotos commented May 24, 2023

I have the Models:
ZipCode

  • name

Place

  • name
  • zipCodes O2M

A Place can have one or more ZipCodes
Place.zipCodes is therefore to be retrieved as an array.

So far it works fine.
If I do not assign any ZipCode to a Place then the Place is retrieved with an empty array. This is exactly how I would expect it to behave.

But the type is generated like:
Place.zipCodes?: Maybe<Array>...

Which marks the field as "undefined or null" I guess this will never occur in reality, but the generated types of graphQL-codegen are not nice to work with now.

Also if I define a query like do:
Place(sort: "name") { id name zipCodes { id } }

Here it is guaranteed that the zipCodes Array will at leat be defined and possibly an empty array.
The typing-system insists to test for null and undefined and forces the use of ?-operator.

ScreenShot of Generated Type:
Untitled 2

ScreenShot of usage:
Untitled 3

ScreenShot what Data is fetched: empty Array, and array with content
Untitled 4

@abdonrd
Copy link
Contributor

abdonrd commented Jan 27, 2024

Something related to this...

When I'm using Codegen, the arrays come as (Item | null)[] | null instead of Item[]

This makes it difficult to iterate the array, because TypeScript forces you to add the !:

const CompetitionQuery = gql`
  query {
    competition: competitions_by_id(id: "002756fa-8e5b-4e8e-9e44-6fb80d7ed867") {
      id
      slug
      name
      editions {
        id
        slug
        name
      }
    }
  }
`;

const [result] = useQuery({
  query: CompetitionQuery,
});

// ⚠️ here `editions` appear as possible null
const editionNames = result.competition?.editions!.map((edition) => {
  // ⚠️ here `edition` appear as possible null
  edition!.name
});

I would expect that editions and edition could not be null, so:

const editionNames = result.competition?.editions.map(({name}) => name);

@evangeloszotos
Copy link

Yeah, both.

You expect both not to be null.

  • Editions - Array
  • Edition - Item

So do I.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 📋 Backlog
Development

Successfully merging a pull request may close this issue.