Skip to content

Fix(core): Type definition for GraphQLRequestParams#3807

Merged
JoviDeCroock merged 2 commits intourql-graphql:mainfrom
arkandias:fix/graphql-request-params-type
Aug 9, 2025
Merged

Fix(core): Type definition for GraphQLRequestParams#3807
JoviDeCroock merged 2 commits intourql-graphql:mainfrom
arkandias:fix/graphql-request-params-type

Conversation

@arkandias
Copy link
Copy Markdown
Contributor

Summary

The current implementation makes the field variables optional if and only if at least one field of Variables is void | null.

type GraphQLRequestParams<
  Data = any,
  Variables extends AnyVariables = AnyVariables,
> =
  | ({
      query: string | Data;
    } & (Variables extends void
      ? {
          variables?: Variables;
        }
      : Variables extends {
            [P in keyof Variables]: Exclude<Variables[P], null | void>;
          }
        ? Variables extends {
            [P in keyof Variables]: never;
          }
          ? {
              variables?: Variables;
            }
          : {
              variables: Variables;
            }
        : {
            variables?: Variables;
          }))
  | {
      query: string | Data;
      variables: Variables;
    };

Instead, it should make the field variables optional if and only if all the fields of Variables are either optional or can be null | void:

export type GraphQLRequestParams<
  Data = any,
  Variables extends AnyVariables = AnyVariables,
> =
  | ({
      query: DocumentInput<Data, Variables>;
    } & (Variables extends void
      ? {
          variables?: Variables;
        }
      : Variables extends {
            [P in keyof Variables]: undefined extends Variables[P]
              ? unknown
              : null extends Variables[P]
                ? unknown
                : void extends Variables[P]
                  ? unknown
                  : never;
          }
        ? {
            variables?: Variables;
          }
        : {
            variables: Variables;
          }))
  | {
      query: DocumentInput<Data, Variables>;
      variables: Variables;
    };

You can check both behaviors here.

In particular, you'll see:

const foo1: GraphQLRequestParams<{a: string; b: number | null}> = {}; // No error, although field `a` is required and non-nullish
const foo2: FixGraphQLRequestParams<{a: string; b: number | null}> = {}; // Error, as expected
const foo3: FixGraphQLRequestParams<{a?: string; b: number | null; c: boolean | undefined; d: string | void}> = {}; // No error -- all fields optional or nullish

Set of changes

Just fixed the type definition.

This should not be a breaking change as it only affect typing, although it may cause compilation errors for code that was previously (incorrectly) allowed to omit required variables.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Aug 7, 2025

🦋 Changeset detected

Latest commit: 6327e10

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@urql/core Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Signed-off-by: Julien Hauseux <julien.hauseux@gmail.com>
@arkandias arkandias force-pushed the fix/graphql-request-params-type branch from 966f8d6 to 5a94c5e Compare August 7, 2025 23:38
@arkandias
Copy link
Copy Markdown
Contributor Author

Just added a changeset: In my opinion this should be merely a minor bump.

@JoviDeCroock JoviDeCroock merged commit b757332 into urql-graphql:main Aug 9, 2025
13 checks passed
@github-actions github-actions bot mentioned this pull request Aug 9, 2025
@arkandias arkandias deleted the fix/graphql-request-params-type branch August 9, 2025 13:47
arkandias added a commit to arkandias/urql that referenced this pull request Aug 10, 2025
)

Signed-off-by: Julien Hauseux <julien.hauseux@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants