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

[unused-fields] Add an ignoredFields option #122

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

azz
Copy link

@azz azz commented Sep 24, 2021

👋 Hey team.

Context

In our codebase, we use connections pretty extensively with graphql-ruby. Because there's a lot of nullability in our graph, we have a library function to extract all the nodes from a connection. For example:

const {bars} = useFragment(graphql`
  fragment Foo_foo on Foo {
    bars {
      edges {
        node {
          name
        }
      }
    }
  }
`, props.foo);

// getNodes will grab all the nodes from `bars` filtering out nulls etc.
getNodes(bars).map(bar => <div>{bar.name}</div>);

As you can imagine, this yields a bunch of unused-field warnings on the generic edges and node fields across a whole bunch of files.

Solution

I'm proposing an option to unused-fields that allows us to suppress warnings for these common fields, like so:

  'relay/unusued-fields': ['warn', {ignoredFields: ['node', 'edges']}]

I've raised a PR with specs and docs. Hope you're happy to 🚢 !

@azz azz changed the title Add an ignoredFields option for relay/unused-fields [unused-fields] Add an ignoredFields option Sep 24, 2021
@azz
Copy link
Author

azz commented Dec 2, 2021

Hey, @kassens, sorry to ping direct, but any chance of getting this shipped?

@bonham000
Copy link

Would be great to see a feature like this. We have a codebase where we have a lot of id fields which we need to query but are not used directly in the components. It would be nice to just ignore linting for id fields.

@acconrad @kassens any update here?

@muhajirdev
Copy link

in case anyone want to use this feature before this PR got merged. yarn patch eslint-plugin-relay
just google yarn patch

@rbalicki2
Copy link

Sorry about the late replies!

  • @bonham000id fields are automatically selected in all queries. Are you sure you need to select them? Check the generated file (FooQuery.graphql.js or what-have-you) for the query text, it should include id everywhere.
  • @muhajirdev — this seems like a great use case for Relay resolvers. Have you looked into those? https://relay.dev/docs/guides/relay-resolvers/ (One thing that is annoying here is that there are no generics, so e.g. you wouldn't be able to define a single resolver for all connections... sorry :/)

Let me know what you think! I'm not opposed to merging something like this, but IMO we should try to fix whatever issue we have with Relay, as it should be pretty rare that you need to select fields you don't use. (Or that eslint isn't smart enough to figure out that you're using.)

@michael-haberzettel
Copy link

michael-haberzettel commented Oct 28, 2022

Relay resolvers are great but they are still experimental even in last Relay version. We have a similar issue in our codebase with edges and node, this could be great if this option can be used.

This PR #95 seems related to same the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants