0no-co / gql.tada

šŸŖ„ Magical GraphQL query engine for TypeScript
https://gql-tada.0no.co
MIT License
2.54k stars 42 forks source link

graphql() function duplicates backslashes in query #379

Closed mr-zwets closed 3 weeks ago

mr-zwets commented 3 weeks ago

Describe the bug

The issue

I ran into a bug where some my graphql queries in browser returned empty results, by investigating the 'Network' tab in the console I found the issue was duplicate backslashes.

this is the network query I would see, notice the 8 backslashes after token_category: {_eq:

{"query":"{\n  output(\n    where: {token_category: {_eq: \n\"\\\\\\\\xa444197a63361d04ebd13afe737c9210ea9df1403807a7f67bf90647ff79b75b\"}, _and: {nonfungible_token_capability: {_eq: \n\"none\"}, locking_bytecode: {_eq: \n\"\\\\x76a9148ee26d6c9f58369f94864dc3630cdeb17fae2f2d88ac\"}}, _not: {spent_by: {}}}\n  ) {\n    transaction_hash\n    locking_bytecode\n    nonfungible_token_commitment\n    __typename\n  }\n}","variables":{}}

this would return { outputs: [] } because of the invalid token_category that is being queried

My Usage

the function which generates this query is

import { graphql } from "gql.tada";

export async function queryUserReceipts(
  chaingraphClient,
  poolTokenId : string,
  userLockingBytecode:string
) {

  const query = graphql(`query {
    output(
      where: {
        token_category: {
          _eq: "\\\\x${poolTokenId}"
        }
        _and: {
          nonfungible_token_capability: { _eq: "none" }
          locking_bytecode: { _eq: "\\\\x${userLockingBytecode}" }
        }
        _not: { spent_by: {} }
      }
    ) {
      transaction_hash
      locking_bytecode
      nonfungible_token_commitment
    }
  }`);
  return (await chaingraphClient.query(query, {})).data
}

I need 2 backslashes in front of the token_category bytea type which is need so escape in JS with 2 more backslashes resulting in _eq: "\\\\x${poolTokenId}"

Cause

I had the issue both when using the Urql client a well as graphql-request, so from this it seemed it was not client specific.

I was able to fix the issue by swapping the { graphql } from "gql.tada"; by import { gql } from "@urql/core";

import { gql } from "@urql/core";

So I believe there is a backslash duplication edgecase that happens sometimes because of gql-tada's graphql()

Reproduction

I still have to make a repo for the reproduction of the issue

Reproduction

No response

gql.tada version

"gql.tada": "^1.8.2"

Validations

kitten commented 3 weeks ago

I traced the bug down to a typo in graphql.web. Basically this will work the first time around but fail on subsequent parse runs due to a typo in a regex.

That said, while it's great that you caught this, also a word of warning: Don't create queries dynamically. Pass variables instead. While this is might partially work both in the string parsing of gql.tada and in graphqlsp, if I'm not mistaken it's not explicitly supported and you lock yourself out of the benefits of having statically composable and persist-able queries.

mr-zwets commented 3 weeks ago

Hi! Thanks so much for swiftly looking into this and identifying the issue even before I had a reproducible build!

really appreciate it. šŸ™ Thanks for the feedback on my Gql queries, I'm quite a new user.

I have updated my code to not create the queries dynamically which fixed the issue šŸ˜… :

export async function queryUserReceipts(
  chaingraphClient,
  poolTokenId : string,
  userLockingBytecode:string
) {
  const query = graphql(`query userStakingReceipts (
    $poolTokenId: bytea,
    $userLockingBytecode: bytea
  ) {
    output(
      where: {
        token_category: {
          _eq: $poolTokenId
        }
        _and: {
          nonfungible_token_capability: { _eq: "none" }
          locking_bytecode: { _eq: $userLockingBytecode }
        }
        _not: { spent_by: {} }
      }
    ) {
      transaction_hash
      locking_bytecode
      nonfungible_token_commitment
    }
  }`);
  const variables = {
    poolTokenId: `\\x${poolTokenId}`,
    userLockingBytecode: `\\x${userLockingBytecode}`
  }
  return (await chaingraphClient.query(query, variables)).data
}

I did notice that in variables it worked in Node with 4 backslashes but for web usage I needed to use only 2 (which also worked in the Node environment)

I don't love the fact that i have to name the whole function, the query constant and the query string itself but if that is the best practice then i will do it this way šŸ‘

kitten commented 3 weeks ago

It's a lot easier to keep track of queries that way, and ensures that you'll never interpolate something that GraphQLSP or gql.tada can't interpret (well, the latter would be quite obvious since the types would then not be inferred šŸ˜…)

But my takeaway here is that GraphQLSP should maybe have a warning for more complex cases. This one is admittedly easy to deal with, but still a little dangerous since we don't know if that string is escaped during the type checking phase.

Anyway, just thought I'd point it out