walmartlabs / lacinia

GraphQL implementation in pure Clojure
http://lacinia.readthedocs.io/en/latest/
Other
1.82k stars 163 forks source link

`fn?` spec for resolver functions is too tight. What about `ifn?` #291

Closed orestis closed 5 years ago

orestis commented 5 years ago

After upgrading to Lacinia 0.33, my schema fails to validate. The reason is that I'm using vars instead of functions as my resolvers, thus allowing me to just reevaluate my resolver in the REPL, without having to stop/start my entire system.

Since vars are callable like functions, this used to work perfectly in 0.32, but now the spec is too tight. I think using ifn? as the spec predicate would allow me to use vars as resolvers while still capturing the intent of the validation.

hlship commented 5 years ago

fn? is too loose, but I think we may want to keep it very strict: (or (fn? %) (var? %)) because otherwise, a keyword (which returns true from ifn?) works, and that would mean that we would no longer catch when keywords were not replaced with actual resolvers.

hlship commented 5 years ago

But absolutely, vars should be allowed, to support REPL oriented development.

orestis commented 5 years ago

Thanks for the quick response! I noticed that multimethods also fail the fn? check -- though I'm not sure exactly how a multimethod could practically work as a resolver.

hlship commented 5 years ago

What would you dispatch on, especially at the root? But if you really want a multi-method, you could wrap the multi-method with a plain function, then use that plain function all over your schema, if you like. I don't see the point, myself.

orestis commented 5 years ago

Fair point. I was just trying to think of other cases were fn? was not applicable.

24 Ιουν 2019, 6:23 μμ, ο χρήστης «Howard M. Lewis Ship notifications@github.com» έγραψε:

What would you dispatch on, especially at the root? But if you really want a multi-method, you could wrap the multi-method with a plain function, then use that plain function all over your schema, if you like. I don't see the point, myself.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.