ID scalar should parse integers into int, not string

[Fontinalis]

The ID scalar type represents a unique identifier, often used to refetch an object or as key for a cache. The ID type appears in a JSON response as a String; however, it is not intended to be human-readable. When expected as an input type, any string (such as \"4\") or integer (such as 4) input value will be accepted as an ID.

The ID scalar type accepts string and integer, but in the ParseLiteral function, it returns everything as a string, even the integers.

var ID = NewScalar(ScalarConfig{
	Name: "ID",
	Description: "The `ID` scalar type represents a unique identifier, often used to " +
		"refetch an object or as key for a cache. The ID type appears in a JSON " +
		"response as a String; however, it is not intended to be human-readable. " +
		"When expected as an input type, any string (such as `\"4\"`) or integer " +
		"(such as `4`) input value will be accepted as an ID.",
	Serialize:  coerceString,
	ParseValue: coerceString,
	ParseLiteral: func(valueAST ast.Value) interface{} {
		switch valueAST := valueAST.(type) {
		case *ast.IntValue:
			return valueAST.Value
		case *ast.StringValue:
			return valueAST.Value
		}
		return nil
	},
})

I think we should change that and resolve it like in the Int scalar's ParseLiteral, so a solution would look like this

func(valueAST ast.Value) interface{} {
	switch valueAST := valueAST.(type) {
	case *ast.IntValue:
		if intValue, err := strconv.Atoi(valueAST.Value); err == nil {
			return intValue
		}
	case *ast.StringValue:
			return valueAST.Value
	}
	return nil
}

[Fontinalis]

/cc @chris-ramon @miiton @egonelbre

[limoli]

I would add an unexpected behavior that I have just detected using ID scalar as nullable. If you have a pointer of type uint64 (*uint64) and you want to expose that value using ID scalar, GraphQL doesn't evaluate the real value of that pointer, but instead, it returns a string value with the pointer address.

Expected: userId: 1

Got userId: "0xc000476c50"

[limoli]

Any news about this issue? @Fontinalis @miiton @egonelbre It is quite important since it is a native GraphQL scalar!

[chris-ramon]

Hi @Fontinalis & @limoli , thanks for bring this up, I think the ID scalar is mean to handled as a string, although in most cases can be a numeric value.

I've went ahead an give a try using the latest graphql-js reference implementation and confirmed that a numeric value for an ID scalar is passed to the resolver as a string value:

const {
  graphql,
  GraphQLSchema,
  GraphQLObjectType,
  GraphQLID,
} = require('graphql');

var schema = new GraphQLSchema({
  query: new GraphQLObjectType({
    name: 'RootQueryType',
    fields: {
      check: {
        type: GraphQLID,
        args: {
          id: {
            type: GraphQLID,
          },
        },
        resolve(source, { id }) {
          console.log("arg id:", typeof id); // arg id: string
          return id;
        },
      },
    },
  }),
});

var query = '{ check(id: 10) }';

graphql(schema, query).then((result) => {
  const { data } = result;
  console.log("resolved id:", typeof data.check); // resolved id: string
});

Said that, the previous behavior matches with the graphql-go implementation:

package main

import (
	"encoding/json"
	"errors"
	"fmt"
	"log"
	"reflect"

	"github.com/graphql-go/graphql"
)

var RootQuery = graphql.NewObject(graphql.ObjectConfig{
	Name: "RootQuery",
	Fields: graphql.Fields{
		"check": &graphql.Field{
			Type: graphql.ID,
			Args: graphql.FieldConfigArgument{
				"id": &graphql.ArgumentConfig{
					Type: graphql.ID,
				},
			},
			Resolve: func(p graphql.ResolveParams) (interface{}, error) {
				id, ok := p.Args["id"]
				if !ok {
					return nil, errors.New("unexpected arg")
				}
				fmt.Printf("args id: %v \n", reflect.TypeOf(id)) // args id: string
				return id, nil
			},
		},
	},
})

func main() {
	schema, err := graphql.NewSchema(graphql.SchemaConfig{
		Query: RootQuery,
	})
	if err != nil {
		log.Fatal(err)
	}

	query := `{ check(id: 10) }`

	result := graphql.Do(graphql.Params{
		Schema:        schema,
		RequestString: query,
	})

	resultStr, err := json.Marshal(result)

	if err != nil {
		log.Fatal(err)
	}

	fmt.Println(string(resultStr)) // {"data":{"check":"10"}}
}

---

I think if it is require to work with scalar ints, it would be better to use graphql.Int instead of graphql.ID, eg:

package main

import (
	"encoding/json"
	"errors"
	"fmt"
	"log"
	"reflect"

	"github.com/graphql-go/graphql"
)

var RootQuery = graphql.NewObject(graphql.ObjectConfig{
	Name: "RootQuery",
	Fields: graphql.Fields{
		"check": &graphql.Field{
			Type: graphql.Int,
			Args: graphql.FieldConfigArgument{
				"id": &graphql.ArgumentConfig{
					Type: graphql.Int,
				},
			},
			Resolve: func(p graphql.ResolveParams) (interface{}, error) {
				id, ok := p.Args["id"]
				if !ok {
					return nil, errors.New("unexpected arg")
				}
				fmt.Printf("args id: %v \n", reflect.TypeOf(id)) // args id: int
				return id, nil
			},
		},
	},
})

func main() {
	schema, err := graphql.NewSchema(graphql.SchemaConfig{
		Query: RootQuery,
	})
	if err != nil {
		log.Fatal(err)
	}

	query := `{ check(id: 10) }`

	result := graphql.Do(graphql.Params{
		Schema:        schema,
		RequestString: query,
	})

	resultStr, err := json.Marshal(result)

	if err != nil {
		log.Fatal(err)
	}

	fmt.Println(string(resultStr)) // {"data":{"check":10}}
}