Khan / genqlient

a truly type-safe Go GraphQL client
MIT License
1.07k stars 107 forks source link

String interpolation #225

Closed indrora closed 2 years ago

indrora commented 2 years ago

Is your feature request related to a problem? Please describe.

Sometimes, I need to pass in a variable that will be appended/inserted in the middle of a string, such as a free text search. Unfortunately, this isn't easily possible, so I end up writing wrapper queries to do the heavy lifting for me:

query searchIssues($query:String!, $cursor:String) {
    search(
        type:ISSUE
        query: $query
        first: 10
        after:$cursor
        ) {
        issueCount
        issues:nodes {
          ...
       }
    }
}

Describe the solution you'd like

query getX($assign:String!) {
  search(type:ISSUE, query:"assignee:{{$assign}} -tag:cheesecake") {
     # ...
  }
}
benjaminjkraft commented 2 years ago

Thanks for raising this! I'm not sure I quite understand how genqlient should help here; it seems to me like simply writing the first query, having genqlient generate a helper for that, then wrapping the helper in a function that does the string interpolation would both work and be a good division of responsibilities. Is there a reason I'm not seeing why it's helpful for genqlient to do this?

In general, while in theory genqlient could add a facility for interpolating strings in variables, I'm hesitant to do so both because it diverges from standard GraphQL and because string interpolation without escaping is dangerous territory (and escaping is impossible without knowing the syntax of the string).

indrora commented 2 years ago

Being able to template strings (and mark those strings/inputs as templated) means i can keep the type safety of inputs without having to write extra code. For instance:

query MyQuery(
  #@graphqlient(template:"I want {{.Input}} jugs of liquor {{if bananas}}and bananas{{end}}")
  $query:string
  #@graphqlient(OmitTemplated)
  $input:int
  $bananas:bool) {
  someFnQUery(query: $query) {
     jugs
     bunches(bananasIncluded:$bananas) {
       ... on OatBunch { ... }
       ... on DudeBunch { ... }
       ... on BananaBunch { ... )
     }
  }
}

Now when I go to make that call, the string $query is filled in. This is the logical equivalent to me writing


query MyQueryFn($query:string, $bananas: bool) {
  someFnQuery($query) {
    jugs
    bunches(includeBananas:$bananas) {
      .. on OatBunch {...}
     ... on ...
  }
}

and then writing a function around it...

import "text/template"

func MyRealQueryFn(input string, bananas boolean) (MyQueryFnOutput, error) {
   tmpl, err := template.New("query").Parse("I want {{.Input}} jugs of liquor {{if bananas}}and bananas{{end}}")
   type MyQueryFnInput struct { Input String, Bananas bool }
   if err != nil {
      return nil, err
   }

   // fill the template
   s := ""
   b := bytes.NewBufferString(s)

   err = tmpl.Execute(b, MyQueryFnInput{Input: input, Bananas: bananas})
   if err != nil { return nil, err }
   return MyQueryFn(s, bananas)
}

There's already information being added into the graphql model (omitempty,pointer,etc) -- Being able to say "this variable is in the template" and "This variable needs to be preprocessed" would be nice.

benjaminjkraft commented 2 years ago

Thanks for the additional detail. At this point I still don't really see enough benefit to having genqlient integrate Go templating in this way to justify the cost that any new feature adds. I think the integration would actually be somewhat complicated (how do we know what type input should be? keep in mind text/template is neither explicitly typed nor type-checked at compile/generate time) and it seems like it can be done just as easily by the user in the method you describe. (You could write some helpers to deal with the boilerplate, if you find this coming up a lot.) Or, you could write your own library that uses both text/template and genqlient in this way (perhaps even parsing out its own magic-comments from the queries; just use any prefix other than @genqlient and genqlient will ignore them); if it proves to be broadly useful we could reconsider but I just don't think it makes sense at this time.