頼むからGraphQLのクエリを動的に組み立てるのはやめてくれ

主にapolloの話。

https://github.com/Shopify/graphql-js-client のようなクエリビルダを使ってるんだったら別。

ここでいう「動的に」というのは、下記のように通常の文字列結合で引数とかselection setを突っ込むようなコードを指している。

apolloClient.query(gql`
  query {
    procucts(${variables}) {
      id
    }
  }
`);

当たり前なんだけど、variables に "first: 10) { __typename } users(first: 10" みたいな文字列を突っ込んだら、想定とは全く異なるクエリになって何が起こるかわかったもんじゃない。

変数はGraphQLのvariablesに格納するようにしろ。

apolloClient.query(gql`
  query MyQuery($first: Int!) {
    procucts(first: $first) {
      id
    }
  }
`, { variables: { first } }));

要は クエリは静的に決定された状態にしておけ というのが言いたいだけなんだけど、実行結果が想定外になる、というだけじゃなく、他にも弊害が色々ある。

例えば冒頭のコード、gql というTagged Templateを使ってて、apolloでコード書いたらよく出てくるんだけど、こいつは中身の文字列をparseしてGraphQLのASTに変換する役割を持っている。gql は一度parseしたASTは元のクエリ文字列をキーにしたObjectに格納してcacheとして利用するように実装されている。折角cacheを持っているのにそこにhitしなかったらメモリの無駄、というのもあるし、このcacheはただObjectにガンガン値突っ込んでるだけで、LRUとかそういうことは一切していないので、SSRの文脈でこのコードをNode.jsのサーバーで動かしたらメモリリークを引き起こす。

クエリの文字列をメモリ上に持ち続ける、というのは persisted queryの文脈などでも出てくる。この場合はクエリ文字列のhash値をキーに、parseしたASTやクエリ文字列を値にした辞書を中間層に保持することになるが、もしクエリが動的にコロコロ変わるようになっていたら、こういった仕組みだってやはり真当に動作しない。

大概のケースだと、codegenとかその手の静的な解析ツールを書けたときにこういう行儀の悪いコードは見つけられると思っていたけど、 apollo client:codegen コマンドだと特にエラーも警告も吐かずにそれっぽい型ファイルを生成してしまうのも質が悪い。