-
Notifications
You must be signed in to change notification settings - Fork 108
Add ignore config to no-unused-fields
rule
#2281
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
Add ignore config to no-unused-fields
rule
#2281
Conversation
🦋 Changeset detectedLatest commit: 0169da2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
type: 'object', | ||
additionalProperties: false, | ||
properties: { | ||
ignoredFieldSelectors: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few other names I considered here were:
allowedUnusedFields
allowedUnusedFieldSelectors
ignoredFields
I think ignoredFieldSelectors
avoids some of the oxymoronic qualities of allowedUnused*
and provides useful context that this configuration uses ESLint selectors, but am open to other options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer ignoredFieldSelectors
too
description: [ | ||
'Fields that will be ignored and are allowed to be unused.', | ||
'', | ||
'> These fields are defined by ESLint [`selectors`](https://eslint.org/docs/developer-guide/selectors). Paste or drop code into the editor in [ASTExplorer](https://astexplorer.net) and inspect the generated AST to compose your selector.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The naming-convention
rule already makes use of ESLint selectors within configurations and it felt good to follow suit here. While this adds some clunkiness to the configuration (allowing an individual field looks like "FieldDefinition[name.value=field]"
instead of "field"
), there are benefits in that allowing fields with wildcards, based on their parent object, etc. are implicitly supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine to use selectors here instead of plain strings because we have the flexibility to choose parent name and parent pattern, see
[
'[parent.name.value=PageInfo][name.value=endCursor]',
'[parent.name.value=PageInfo][name.value=startCursor]',
'[parent.name.value=PageInfo][name.value=hasNextPage]',
'[parent.name.value=PageInfo][name.value=hasPreviousPage]',
'[parent.name.value=/Edge$/][name.value=cursor]',
'[parent.name.value=/Connection$/][name.value=pageInfo]',
]
we can specify ends with Edge
and Connection
, @maciesielka maybe we should update the default options to include it there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for taking time merging it, great work!
Description
Changes here add a
ignoredFieldSelectors
configuration to theno-unused-fields
rule such that some fields are allowed to be unused.Fixes #2280
Type of change
Please delete options that are not relevant.
Checklist:
CONTRIBUTING doc and the
style guidelines of this project