Skip to content

[unused-fields] Add ignore fields option #94

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

Closed
wants to merge 3 commits into from

Conversation

janicduplessis
Copy link
Contributor

Motivation

I'm trying to enable this rule in my codebase but a few patterns provide a lot of false positives. An example is when using from React Native, it expects nodes to have an id prop but this is only accessed internally in the FlatList code and not in the current module. Although it is probably possible to use fragments here I think adding more flexibility to the rule by specifying ignored fields can help adopting it.

I was wondering if we should include __typename as part of the default ignoredFields and remove the hardcoded check or keep it like this. The only difference it would make is if you specify ignoreFields and relied on __typename being ignored then you'd have to add it in your ignoreFields list.

Test plan

Tested on a large relay codebase and added tests.

@janicduplessis janicduplessis changed the title Add ignore fields option to unused-fields rule [unused-fields] Add ignore fields option May 16, 2020
Copy link

@AugustoCalaca AugustoCalaca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be great to see this merged

@poteto
Copy link
Contributor

poteto commented Feb 5, 2021

You should now be able to suppress these by adding a # eslint-disable-next-line relay/unused-fields comment in 1.8.2. Feel free to reopen if that doesn't address your use case.

@poteto poteto closed this Feb 5, 2021
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.

5 participants