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

[Bug Reproduce] Can't use undefined as enum value #1367

Closed

Conversation

IvanGoncharov
Copy link
Member

Extracted from #1267

From #836 it looks like both undefined and NaN could be used as an enum values:

const TestEnum = new GraphQLEnumType({
  name: 'TestEnum',
  values: {
    UNDEFINED: { value: undefined },
    NAN: { value: NaN },
  },
});

And run this query:

{
  undefined: fieldWithEnumInput(input: UNDEFINED)
  NaN: fieldWithEnumInput(input: NAN)
}

It will result in:

{
  "data": {
    "undefined": null,
    "NaN": null
  },
  "errors": [
    {
      "message": "Argument \"input\" has invalid value UNDEFINED.",
      "path": [ "undefined" ]
    }
    {
      "message": "Argument \"input\" has invalid value NAN.",
      "path": [ "NaN" ]
    }
  ]
}

**Extracted from graphql#1267**

From graphql#836 it looks like both `undefined` and `NaN` could be used as an enum values:
```js
const TestEnum = new GraphQLEnumType({
  name: 'TestEnum',
  values: {
    UNDEFINED: { value: undefined },
    NAN: { value: NaN },
  },
});
```
And run this query:
```graphql
{
  undefined: fieldWithEnumInput(input: UNDEFINED)
  NaN: fieldWithEnumInput(input: NAN)
}
```
It will result in:
```json
{
  "data": {
    "undefined": null,
    "NaN": null
  },
  "errors": [
    {
      "message": "Argument \"input\" has invalid value UNDEFINED.",
      "path": [ "undefined" ]
    }
    {
      "message": "Argument \"input\" has invalid value NAN.",
      "path": [ "NaN" ]
    }
  ]
}
```
Copy link
Contributor

@mjmahone mjmahone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why, but this is now causing test failures. I might be able to dig in later today.

@IvanGoncharov
Copy link
Member Author

@mjmahone Yes it known issue with valueFromAST since it uses undefined return to indicate errors.
Ideally, it should be designed similar to coerceValue:

type CoercedValue = {|
+errors: $ReadOnlyArray<GraphQLError> | void,
+value: mixed,
|};
type Path = {| +prev: Path | void, +key: string | number |};
/**
* Coerces a JavaScript value given a GraphQL Type.
*
* Returns either a value which is valid for the provided type or a list of
* encountered coercion errors.
*
*/
export function coerceValue(
value: mixed,
type: GraphQLInputType,
blameNode?: ASTNode,
path?: Path,
): CoercedValue {

Correctly handling undefined is minor use case for breaking change but it will also makes errors to contain explanation why certain literal value is invalid which is pretty important.

This PR is [Bug Reproduce] just to document failing use cases until it will be fixed.

@IvanGoncharov IvanGoncharov changed the title [Bug Reproduce] Can't use undefined and as enum value [Bug Reproduce] Can't use undefined as enum value Sep 1, 2019
IvanGoncharov added a commit to IvanGoncharov/graphql-js that referenced this pull request Sep 2, 2019
@IvanGoncharov
Copy link
Member Author

Resolved in #2148

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants