graphql-rust / graphql-client

Typed, correct GraphQL requests and responses in Rust
Apache License 2.0
1.15k stars 158 forks source link

Invalid caching of raw query with `selected_operation` #221

Open theduke opened 5 years ago

theduke commented 5 years ago

Currently, using the same file and selected_operation in the custom derive does not work.

The generated const QUERY = ""... is the same string for all the generated modules, re-using the value of the first generated.

Eg:

mutation Login($account: String!, $user: String!, $password: String!) {
    login(account: $account, user: $user, password: $password) {
        account {
            id
            identifier
            name
        }
        user {
            id
            username
            email
        }
        token
    }
}

mutation Signup($data: SignupInput!) {
    signup(data: $data) {
        account {
            id
            identifier
            name
        }
        user {
            id
            username
            email
        }
        token
    }
}

mutation BookmarkCreate($url: String!, $title: String!, $description: String) {
    bookmarkCreate(url: $url, title: $title, description: $description) {
        ...Bookmark
    }
}

fragment Bookmark on Bookmark {
    id
    url
    title
    description
}

query Bookmarks {
    bookmarks {
        ...Bookmark
    }
}

mutation Login($account: String!, $user: String!, $password: String!) {
    login(account: $account, user: $user, password: $password) {
        account {
            id
            identifier
            name
        }
        user {
            id
            username
            email
        }
        token
    }
}

mutation Signup($data: SignupInput!) {
    signup(data: $data) {
        account {
            id
            identifier
            name
        }
        user {
            id
            username
            email
        }
        token
    }
}

mutation BookmarkCreate($url: String!, $title: String!, $description: String) {
    bookmarkCreate(url: $url, title: $title, description: $description) {
        ...Bookmark
    }
}

fragment Bookmark on Bookmark {
    id
    url
    title
    description
}

query Bookmarks {
    bookmarks {
        ...Bookmark
    }
}

Result:

mod login {
  const QUERY: &'static str = "mutation Login ....";
}
mod signup {
  // Query re-used !
  const QUERY: &'statc str = "mutation Login...";
}
tomhoule commented 5 years ago

Sorry for the delay replying to this. My understanding of the problem is that sending the entire document is the right behaviour. We send operationName as part of actual request for the server to pick one operation from the document. This is how I understand the references to operationName in the spec, e.g. https://facebook.github.io/graphql/June2018/#sec-Executing-Requests

Of course we could parse the query and emit only the operation we want (potentially minified) as a size optimization, but I think the current behaviour is correct.

theduke commented 5 years ago

Mhm, I was definitely getting the wrong response data for the first defined query though which is the only reason I checked the generated code.

Maybe the wrong operationName was passed.

tomhoule commented 5 years ago

That could be the case, I'll check if the logic there is sound. Marking this as a bug for now.

tomhoule commented 5 years ago

We should pass the right operationName, but if that is not enough it wouldn't be too hard to filter out not-selected operations from the document (keeping the selected one and fragments) and send that in the request. I'll work on this in priority as soon as I have the time.

tomhoule commented 5 years ago

hmmm, it looks like OPERATION_NAME is defined multiple times in modules generated through the CLI, but this issue is about the custom derive API, I'll continue investigating

tomhoule commented 5 years ago

226 tests that we are sending the right operation name with derive queries with selected_operation. Still investigating.

mathstuf commented 5 years ago

Note that schema element usage is tracked due to #116. The same could be done for the query side as well.

mathstuf commented 5 years ago

Hmm. At least Github tends to give back line/column information based on the query sent. Right now, that corresponds with the actual query file, but trimming down would mean that it doesn't. Replacing unused structure lines with empty lines would allow the line numbers to still correspond. But this then loses on deduplication of static data and may increase binary size (granted, a small issue).