Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graphiql tries to read string as integer and fails since it starts with 07 #89

Closed
buma opened this issue Feb 4, 2016 · 3 comments
Closed

Comments

@buma
Copy link

buma commented Feb 4, 2016

I am using java-graphql and graphiql for testing. I added custom graphql type for time which is represented as string in GraphQL but problem is that GraphiQL fails when parsing introspection query since it seems to try to read defaultValue it as integer and fails. Since I get an error:

Syntax Error GraphQL (1:2) Invalid number, unexpected digit after 0: "7".

1: 07:30

It works correctly in "toTime" which has defaultValue 10:00. And it also works correctly if defaultValue is "7:30".
This is part of introspection response:

{
                  "name": "fromTime",
                  "description": "The beginning of the departure window, in HH:MM",
                  "type": {
                    "kind": "SCALAR",
                    "name": "String",
                    "ofType": null
                  },
                  "defaultValue": "07:30"
                },

And link to full response.

@asiandrummer
Copy link
Contributor

@buma so I've tried to reproduce with example app, and got this:

defaultValue: ""07:30"" // notice the difference here
description: "test string time"
name: "time"
type: {
  kind: "SCALAR"
  name: "String"
  ofType: null
}

I was thinking this might be a difference between how GraphQLInputObjectType is build in graphql-java and graphql-js - @leebyron would you have an idea what's happening here?

@buma
Copy link
Author

buma commented Feb 4, 2016

Interesting. @andimarek any idea if this is graphql-java bug or bug in my code?

GraphQLFieldDefinition planField = GraphQLFieldDefinition.newFieldDefinition()
            .name("plan")
            .description("Gets plan of a route at a specific time")
            .type(profileResponseType)
            .argument(GraphQLArgument.newArgument().name("from")
                .type(new GraphQLNonNull(inputCoordinateType)).build())
            .argument(GraphQLArgument.newArgument()
                .name("to")
                .type(new GraphQLNonNull(inputCoordinateType))
                .build())
            .argument(GraphQLArgument.newArgument()
                .name("fromTime")
                .type(Scalars.GraphQLString)
                .description("The beginning of the departure window, in HH:MM")
                .defaultValue("07:30")
                .build())
            .argument(GraphQLArgument.newArgument()
                .name("toTime")
                .type(Scalars.GraphQLString)
                .description("The end of the departure window, in HH:MM")
                .defaultValue("10:30")
                .build())
          .build();

@leebyron
Copy link
Contributor

leebyron commented Feb 4, 2016

Thanks for the report! From @asiandrummer's analysis this appears to be a clear bug in graphql-java or at least a shortcoming in the API

http://facebook.github.io/graphql/#sec-The-__InputValue-Type

This part of the spec describes the default value field as being a string encoded in the graphql language. So we can provide numbers, booleans, and strings with "3", "true", and ""str"", respectively.

@leebyron leebyron closed this as completed Feb 4, 2016
acao pushed a commit to acao/graphiql that referenced this issue Jun 1, 2019
Currently, the onlineParser doesn't alter token state for special tokens like `comment` or `invalidchar`. When you load the token state for one of these tokens, it appears to be whatever came before it. When building "intelisense" features for IDEs this can be a complicating factor.

This proposes adding two new special parse rules "Comment" and "Invalid" and altering the parser behavior to detect and insert them when necessary.

This also results in onlineParser becoming more generic with respect to Comment lexer rules. Previously onlineParser knew more than it should about how comments are defined. Now, the provided parse rules (specifically the lexer rules) supply the definition for `Comment`.

Finally, this removes global state from the parser when saving and resuming backup states, keeping that context contained within the parser function. That removes the possibility for poisoning the parse state.
acao pushed a commit to acao/graphiql that referenced this issue Jun 1, 2019
This builds on graphql#89 and graphql#90 to create a more specific parser rule and parser state for NamedType. This makes it easier to builds tools on top of codemirror-graphql which do interesting things when the names of types are encountered, such as jump-to-docs or hover cards.
acao pushed a commit to acao/graphiql that referenced this issue Jun 1, 2019
Since graphql#89, hint no longer shows typeaheads for invalid token states. This uses the technique found in `variables/hint.js` in `getHintsAtPosition`.
acao pushed a commit to acao/graphiql that referenced this issue Jun 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants