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.

Sign up for GitHub

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

Jump to bottom

Not possible to mark parent as @required if parent is nullable due to child being a nullable fragment #1410

Open
joleeee opened this issue Jan 9, 2025 · 9 comments · May be fixed by #1411
Open

Not possible to mark parent as @required if parent is nullable due to child being a nullable fragment #1410

joleeee opened this issue Jan 9, 2025 · 9 comments · May be fixed by #1411

Comments

@joleeee
Copy link

joleeee commented Jan 9, 2025

Describe the bug

TLDR; if a query has a fragment with a @required in a top level field, it bubbles up to the query, but it's not possible to mark the parent of the fragment spread @required.

Example:

I have a fragment

fragment AirportParts on airports @required {
    id
    location @required {
        city
    }
}

And i use it in a query

query MyQuery {
    flights {
        to {
            ...AirportParts @mask_disable
        }
    }
}

Naturally, it leads to a nullable to:

export type MyQuery$result = {
    /**
     * An array relationship
    */
    readonly flights: ({
        /**
         * An object relationship
        */
        readonly to: { // <----- to IS NULLABLE
            readonly id: number;
            readonly iata: string | null;
            /**
             * An object relationship
            */
            readonly location: {
                readonly city: string;
                readonly name: string;
            };
            readonly " $fragments": {
                AirportParts: {};
            };
        } | null; // <----- to IS NULLABLE
    })[];
};

However, it is not possible to mark the to as required:

If you try:

query MyQuery {
    flights {
-        to {
+        to @required {
            ...AirportParts @mask_disable
        }
    }
}
❌ Encountered error in src/routes/.../query.gql
@required may only be used on nullable fields

Reproduction

No response
@AlecAivazis
Copy link
Collaborator

Hm maybe i'm missing something subtle in your issue but you wouldn't need to mark to with @required if the field is non-null. since it would always have a value

@joleeee
Copy link
Author

joleeee commented Jan 10, 2025 •
edited
Loading

Thanks for replying

readonly to: { ... } | null;

means it is typed as nullable

@joleeee
Copy link
Author

joleeee commented Jan 13, 2025

Sorry, but the point was that the field is nullable so i want to add @required, but houdini mistakenly says it is not nullable @AlecAivazis

@joleeee
Copy link
Author

joleeee commented Jan 13, 2025

Commenting out this check makes it work. So it seems there is different logic in checking if @required is valid, and actually applying it.

houdini/packages/houdini/src/codegen/validators/typeCheck.ts

Lines 409 to 416 in d7b30f0

if (!isServerNullable && !isAlreadyClientNullable) {
ctx.reportError(
new graphql.GraphQLError(
`@${config.requiredDirective} may only be used on nullable fields`
)
)
return
}

@SeppahBaws
Copy link
Collaborator

What is the type of that field in your graphql schema?

@joleeee joleeee linked a pull request Jan 13, 2025 that will close this issue
Draft
5 tasks
@joleeee
Copy link
Author

joleeee commented Jan 13, 2025

I think I made a patch that makes it work? But I might've done something weird the the existing logic.

What is the type of that field in your graphql schema?

I'm new to graphql, how can I see this? I use hasura with a postgres backend if it is of any help...

@joleeee
Copy link
Author

joleeee commented Jan 13, 2025

I removed the irrelevant fields

type flights {
  to: airports!
}

type airports {
  id: Int!
  location: locations
}

type locations {
  city: String!
}

@SeppahBaws
Copy link
Collaborator

the to field on flights is non-nullable, so that's why the @required directive cannot be used there.

@joleeee
Copy link
Author

joleeee commented Jan 13, 2025 •
edited
Loading

Yes but ... AirportParts has a @require, which bubbles up to to, which means to becomes nullable, as seen by the typescript output!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

required check accept fragmentspread joleeee/houdini
3 participants
@AlecAivazis @SeppahBaws @joleeee