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

[ESLint-plugin] Add eslint 'graphql/required-fields' rule for 'id' field #166

Merged
merged 1 commit into from
Sep 11, 2020

Conversation

brandon-yetman
Copy link
Contributor

@brandon-yetman brandon-yetman commented Jun 16, 2020

Description

Added id field to graphl/required-fields eslint rule. This rule has been added in Web (https://github.com/Shopify/web/pull/27464) and Plus-Web (https://github.com/Shopify/plus-web/pull/4203) and should be a Shopify-wide rule, given the advantages of always requiring the id field in graphql queries.

A full analysis of the reasoning for the rule, as well as the advantage/risk analysis can be found here.

Changelogs

New rules I enabled

Type of change

  • [ESLint-plugin] Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above (Documentation fix and Test update does not need a changelog as we do not publish new version)

@brandon-yetman brandon-yetman self-assigned this Jun 16, 2020
@brandon-yetman brandon-yetman marked this pull request as ready for review June 16, 2020 14:52
@brandon-yetman brandon-yetman requested review from vsumner and a team June 16, 2020 14:52
@brandon-yetman brandon-yetman merged commit 6d3ec5a into main Sep 11, 2020
@brandon-yetman brandon-yetman deleted the add-graphql-required-field-id branch September 11, 2020 14:40
@vsumner vsumner temporarily deployed to production September 17, 2020 16:50 Inactive
@ismail-syed ismail-syed temporarily deployed to production October 6, 2020 21:20 Inactive
@shopify-shipit shopify-shipit bot deployed to production-postcss-plugin-3 June 23, 2021 13:12 Active
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

4 participants