[Bug Reproduce] Can't use undefined and NaN as enum values

[IvanGoncharov]

I clean up some existing tests but the main purpose of this PR is to report problem with enum values(test case added in 9452661).

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" ]
    }
  ]
}

Because of this check:

graphql-js/src/execution/values.js

Lines 164 to 165 in a62eea8

	const coercedValue = valueFromAST(valueNode, argType, variableValues);
	if (isInvalid(coercedValue)) {

This issue can't be easily fixed because both undefined and NaN are invalid according to isInvalid function.

I would be happy to help with fixing this issue but I don't know how to fix it correctly.
My main question is why valueFromAST can't just throw on errors?

[leebyron]

Good catch - want to factor this into two PRs? The improvements to test files should be mergeable as is. Your last commit testing the edge case enum values should be handled separately. Not sure if valueFromAST should throw or not, but it's clear that it needs some way to indicate coercion failure other than returning undefined

[greysteil]

Hey, maintainer of Dependabot here. Sorry about the reference spam.

I'm going to chat to the GitHub team about whether there's a way to link to an issue / pull from a GitHub pull request description without creating these references. (They come from Dependabot pulling the changelog / release notes into the PR description for the PRs it generates.)

If there's no way that GitHub can let Dependabot do the above, I'll remove the links from the PRs. A pity, but I really don't want to create spam like this!

[mjmahone]

@IvanGoncharov it looks like we can close this, as it was done via two smaller commits instead.

[mjmahone]

Oh wait, you left the test changes in this PR. This looks good to me if you want to fix any conflicts and merge it.

[IvanGoncharov]

Oh wait, you left the test changes in this PR. This looks good to me if you want to fix any conflicts and merge it.

This PR adds test cases that reproducess bug in current implement that prevents NaN and undefined from being used as enum values.
It's not the most critical issue but fixing it involves breaking change to valueFromAST function (throw instead of returning undefined) so I will try fix it on weekends so we can include it in 14.0.0.

[IvanGoncharov]

@mjmahone I agree that this PR looks confusing so will extract failing test case into separate one with proper description.