Add requireResolversForAllFields resolver validation option
#107
Conversation
|
I think this is a great idea, but we'll need to update the docs. The way I'm understanding this option, it is intended to take precedence over the other two resolverValidationOptions, correct? |
|
Yeah, IMO it should take precedence (and not support exclusions via the other options, if present) |
| @@ -279,9 +279,15 @@ function assertResolveFunctionsPresent(schema, resolverValidationOptions = {}) { | |||
| const { | |||
| requireResolversForArgs = true, | |||
| requireResolversForNonScalar = true, | |||
| requireResolversForAllFields = false, | |||
| } = resolverValidationOptions; | |||
There was a problem hiding this comment.
if (requireResolversForAllFields && (! requireResolversForArgs || ! requireResolversForNonScalar)) {
throw new Error(`Can't pass in conflicting options`)
}
|
@nevir I think it would be great if you could make the change that @alexturek proposed, but with a more useful error message that says what exactly the conflicting options are. After that I'll go ahead and merge this. |
|
@nevir are you still working on this? I'd like to merge your PR as soon as it's done! |
|
Sorry! Been distracted by other work; I'll get to this Monday morning |
|
@helfer 👍 |
|
Awesome, thanks! Can you add a test just to make sure the error actually gets thrown in the right conditions? It looks like that's not tested right now. |
|
👍 just pushed some tests |
|
Perfect! We merged some other PR this morning, so you'll have to do another rebase so I can merge it. |
Asserts that _all_ fields have a valid resolve function. This effectively disables default resolver behavior.
|
rebased |
|
Awesome, thanks! |
Asserts that all fields have a valid resolve function. This effectively disables default resolver behavior.
For example, we have a strict set of ACL decorators for our resolvers, and we want developers to be explicit about the access control for each new field they add to our schema
TODO: