Lint for required fields [WIP]

[rgoldfinger]

We have a need to validate that the queries include a uuid field, since that's how we cache things client side. I thought I'd try to make this more generic, as a 'required field' lint rule that errors if the field is 1) present in the schema and 2) not requested in this query.

Is this something that you think is worth including in this package?

Can you offer any guidance on how to pass the rule's options to the rule itself? The rule is invoked within graphql's validate, so I'm not seeing a clear way to do this.

TODO:

• Make sure all of the significant new logic is covered by tests
• Rebase your changes on master so that they can be merged easily
• Make sure all tests pass
• Update CHANGELOG.md with your change
• If this was a change that affects the external API, update the README

[apollo-cla]

@rgoldfinger: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[jnwng]

@rgoldfinger i dont have a particularly strong opinion about this, but it would help me to see a couple of testcases to see how the API would work.

the one time i have seen this is through apollo-client's addTypenameToSelectionSet where they force adding a particular field to the query because they need it for caching as well. however, they can safely assume that __typename exists, whereas you cannot do that for this particular set of things.

anecdotally speaking, we would use this to add id to our queries because we too use that for caching. however, i was planning on implementing it the same way that __typename was (but just thought about the case where id is not a valid field on the selectionSet). i guess my hangup here is that (in the react-apollo case) you're requiring people to specify a field they're not directly using in the component, but is used by the client. adding it as an option here seems okay though as the rules are all opt-in anyway.

[jnwng]

context.options has the options you want

[stubailo]

This sounds good to me - I would love to use this option.

[rgoldfinger]

Hey Jon!

I got it working with the options and added some tests. I'll add some documentation and update the changelog after I get a 👍 on this approach.

We also considered injecting the uuid into the query, but since there's no api for that, this seemed like the easier approach. I agree that devs shouldn't have to worry about how to write their query so that it's cached properly, it should just work. But this seems like a reasonable compromise.

[rgoldfinger]

Ping @jnwng

[jnwng]

@rgoldfinger code looks good, but let me clear up the test issues first (something changed upstream) so you can rebase and we can make sure everything is passing before merging

[jnwng]

re: what we discussed offline, i pushed some changes to master to fix tests, so please rebase and resolve conflicts (you can probably just take my yarn.lock since you dont add / change any dependencies)

[stubailo]

This looks like a great new feature!

[rgoldfinger]

Rebased and updated with documentation

[jnwng]

thank you @rgoldfinger!