RFC: Mark certain queries as used even no attributes are accessed

[wolflu05]

Summary

Checking for unused fields is a really powerful feature which definitely makes sense, but sometimes we don't want that, but instead want to pass the complete return type to an other function which e.g. stores this in local storage, but then we end up with an error that not all properties are accessed. E.g.:

const meQuery = graphql(`
  # we have an error here: Field(s) me.username are not used.
  query me {
    me {
      id
      username
      # ...
    }
  }
`);

const res = await client.query(meQuery, {});
localStorage.setItem("user", res.data!.me);

Proposed Solution

Add some type of @gql-tada-mark-used comment, which could be used like this:

const res = await client.query(meQuery, {});
localStorage.setItem("user", res.data!.me);  // @gql-tada-mark-used

This should then mark everything from the me query as used and there shouldn't be an error

Alternatively a simple @gql-tada-ignore-unused comment to the query definition may be sufficient too.

// @gql-tada-ignore-unused
const meQuery = graphql(`
  # we have an error here: Field(s) me.username are not used.
  query me {

Requirements

Have the ability to mark certain queries as fully used and ignore checking if all fields are actually accessed, because that will fail if the complete object is passed to any function.

[kitten]

Transferred to GraphQLSP, since this is where this is implemented.
This definitely makes sense, especially once we start adding the CLI.

However, I think we may want to first consider expanding bail-out patterns, e.g. when a given field is passed into a function, we should, under some circumstances mark it as used automatically.

[wolflu05]

Thanks for you quick answer and transferring it to this repo, I was not aware that there is a different repo for the LSP.

I'm not sure how much forward through the call tree one could track the usage of the attributes. But in some circumstances one need to save all the attributes, where I think it makes sense to intentionally say that from now on a query should be marked as "all fields used". If following through the call tree to identify used/unused attributes would work, this would conflict with always marking as used if passed to a function, that's why I suggested to intentionally mark it.

[gterras]

I'm trying gql.tada for e2e testing a graphQL API along with supertest, Field(s) 'login.token' are not used. doesn't look like there is an easy way to tell the field is used here:

const query = graphql(`
	query login($email: String!, $password: String!) {
		login(email: $email, password: $password) { 
			token 
		}
	}
`)

const variables: VariablesOf<typeof query> = { email, password }

const { body } = await request(app.getHttpServer())
	.post(GQL_ENDPOINT)
	.send({
		query: print(query),
		variables,
	})
	.expect(200)

return body.data.login.token

[kitten]

@gterras For a field to be marked as used the result type has to flow through to a type in your code. The type checker here can't infer the type of body and it's untyped.

An easy way to fix that is to wrap this request(...).post call in your own helper.

You can adapt this example though from the docs to type your body type: https://gql-tada.0no.co/guides/typed-documents#typeddocumentnode-types

[gterras]

An easy way to fix that is to wrap this request(...).post call in your own helper.

Indeed although I posted this e2e example as a somewhat legitimate use case for needing an escape hatch to that warning, in the sense that adding a wrapper to an 2e2 suite makes it instantly harder to understand, use and maintain.

Like this wrapper would also need a way to inject a token or return errors when needed, and would then need to be tested on its own. I like my tests (especially e2e) to be self-contained and to have zero abstraction (like in the posted example) so that a modified test cannot in any way impact another test.

Also this wrapper requires quite some typing work, and while I don't doubt this is very useful in front-end code, in a testing context it feels like it could be advantageously replaced with a type assertion (since the result will be used only once).

For the record I solved the warning issue by moving the query and its variables to a dedicated file, which makes me think people doing this in the first place would not ever see the warning.

[musjj]

I'm also having this issue when passing the data to a component with a spread operator:

const [{ data }] = useQuery({
  query: FooQuery,
});

if (!data?.foo) {
  return;
}

return <Foo {...data.foo} />;

This should just mark all the fields as used here.

[JoviDeCroock]

@musjj well, we could go that way but imho it's an anti-pattern in GraphQL compared to https://gql-tada.0no.co/guides/fragment-colocation

[musjj]

Thanks for the link, but I'm still very confused. Are there any end-to-end examples showing how this can be put together (still new to GraphQL)? I tried to use fragments like this:

"use client";

import { FragmentOf, graphql } from "@/graphql";
import { useQuery } from "@urql/next";

const UserFragment = graphql(`
  fragment UserFragment on User @_unmask {
    name
    age
  }
`);

export const UserQuery = graphql(
  `
    query UserQuery {
      user {
        ...UserFragment
      }
    }
  `,
  [UserFragment]
);

export function User({ data }: { data: FragmentOf<typeof UserFragment> }) {
  return (
    <article>
      <h1>Name: {data.name}</h1>
      <p>Age: {data.age}</p>
    </article>
  );
}

export function Page() {
  const [{ data }] = useQuery({
    query: UserQuery,
  });

  if (!data?.user) {
    return;
  }

  return (
    <main>
      <User data={data.user} />
    </main>
  );
}

But it still warns that the fields are unused.

EDIT: Alright, it looks like you need to use an unmasked fragment:

-   fragment UserFragment on User @_unmask {
+   fragment UserFragment on User {

Then later unmask it inside the User component:

const user = readFragment(UserFragment, data)

Only then the warning stops.

[malyzeli]

I have two more use-cases where the ability to ignore individual warnings would really help!

---

1. We are testing business logic against the graphql api and storing results as snapshots - from the typesystem point of view fields are marked as unused.

const snapshotQuery = graphql(`
  query currentUser {
    currentUser {
      roles
      availableSections
      mustChangePassword
      email
    }
  }
`);

test.each(users)("User state for %s", async (email) => {
  const client = await createClient(email);
  const data = await client.query(snapshotQuery);
  expect(data).toMatchSnapshot();
});

Yields the following warning:

TS52005: Field(s) 'currentUser.roles', 'currentUser.availableSections', 'currentUser.mustChangePassword', 'currentUser.email' are not used.

---

2. One of our components is smart text input, which apart from basic formatting supports matching some patterns and automatically converting them into metadata, for example "every monday" is stored as DateWithPeriod. For this functionality we are using local store (redux), loading all data when editor opens and sending them back on submit. That means there are no React components directly associated to these queries/fragments and most of it is seen as unused.

const DateWithPeriodFragment = graphql(`
  fragment DateWithPeriod on DateWithPeriod @_unmask {
    startDate
    endDate
    period
  }
`);

Yields the following warning on the fragment:

TS52005: Field(s) 'startDate', 'endDate', 'period' are not used.

And the following when the file is imported from somewhere else:

TS52003: Unused co-located fragment definition(s) "DateWithPeriod" in "./queries"

---

I don't see any appropriate way to fix this - commenting with @ts-expect-error actually doesn't make the warning disappear and I don't want to turn off those diagnostics globally, because they are very useful for the rest of the project.

For now we are just ignoring those warnings but that is definitely not the best developer practise and/or experience.

I'm willing to try implementing a PR, but I would need a bit of guidance how to proceed here, respectively what kind of "mark" would be fitting to suppress the warning. Is it a case for a local directive in graphql block? Can it be solved by comment? I've never worked with language server so I need to do a bit of research...