graphql-rust/graphql-client

Deserializing ID as integer not String

ValouBambou opened this issue · 4 comments

Hi, I'm using an API where ID are integers, not string. Thus, the generated code by the macro is using type ID = String. And it leads to Deserialization error such as Error("invalid type: integer `570721`, expected a string", line: 1, column: 45). Is there a way to specify this to the derive macro ? I'm confused because some of the examples in the documentation are using integers, so it may be possible (EDIT: I realized that the example is confusing because of the hidden context with struct User { id:i32}).

Maybe a simple fix would be to define ID as something like this instead of simply type ID = String in codegen.rs line 56.

enum ID {
    Int(usize),
    String(String),
}

impl From<usize> for ID {
    fn from(value: usize) -> Self {
        ID::Int(value)
    }
}

impl From<String> for ID {
    fn from(value: String) -> Self {
        ID::String(value)
    }
}

// probably also implement ToString trait

Or maybe we could use i128 instead of usize (or even add another variant with i128) since this might be used according to some parts of the Graphql spec:

GraphQL is agnostic to ID format, and serializes to string to ensure consistency across many formats ID could represent, from small auto‐increment numbers, to large 128‐bit random numbers, to base64 encoded values, or string values of a format like GUID.

I could do a PR to implement that if this is a valid solution (but I'm not sure).

I think I remember (but it's a very far away memory) that IDs are defined as strings by the GraphQL spec. We could have a config option for code generation to change that to another type, however.

They are always passed as string as per the spec. You could propose a feature to parse them, but in general I found this is a bad idea.

ID should be opaque for the client and you cannot assume they will have a particular format. Doing otherwise is a risk long term for maintenance. It is also very easy for a provider to provide a "bad" id by error since everything formats to string.

In any case the proposed enum would be a big nono since it would be a breaking change for almost 100% of existing users.

Hey, I opened #476 which should fix this in a backwards compatible way.