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

add "function-required-argument" rule #108

Merged
merged 2 commits into from
Dec 16, 2020

Conversation

sullenor
Copy link
Contributor

@sullenor sullenor commented Dec 9, 2020

Following #106 issue.

This change adds new rule to check missing argument for readInlineData function call. Docs https://relay.dev/docs/en/graphql-in-relay#inline

Also updates .eslintrc.js example in readme.

'use strict';

const utils = require('./utils');
const shouldLint = utils.shouldLint;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe destructure?

@alunyov
Copy link
Contributor

alunyov commented Dec 16, 2020

@sullenor After discussing this issue with my colleagues, they pointed out that the proper fix for the issue should be implemented with the typing for readInlineData. Typesystem should catch these issues (if you don't pass a required second argument to a function).

We should double-check why flow didn't catch this.

@alunyov
Copy link
Contributor

alunyov commented Dec 16, 2020

Ok, I double-checked with the team. We have that rule because flow cannot differentiate between undefined and not specified arguments.

@alunyov alunyov merged commit 5e74bf0 into relayjs:master Dec 16, 2020
@sullenor sullenor deleted the function-required-argument branch December 18, 2020 01:15
@sullenor
Copy link
Contributor Author

sullenor commented Dec 18, 2020

@alunyov thanks for checking and for providing clarity for the flow 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

4 participants