Use {} instead of Object.create(null)

[tedchoward]

Reason for the pull request

I noticed an odd behavior with an app I was working on that uses Apollo. When calling a mutation that takes an object, if I used the variable syntax to send the object, everything worked fine. However, if I passed the object directly as a parameter, I would get this error message: TypeError: Cannot convert object to primitive value, which was thrown deep in request body creation logic of node-fetch.

This works:

mutation CreateThing($thing: ThingInput!) {
  createThing(thing: $thing) {
    success
    message
  }
}

This does not:

mutation CreateThing {
  createThing(thing: {
    thingId: "abc123"
    description: "this is a thing"
  }) {
    success
    message
  }
}

What does this have to do with graphql-js?

As I debugged the two scenarios above, the one difference I discovered is that the thing object passed into my resolver using the object value syntax didn't have a prototype, whereas the thing object sent as a variable did. 🤔 "That's odd," I thought. So I dug further to see how that object was being created. This led me to this project, and valueFromAST(). If the object was passed as a variable, it returned a normal object. If, however, the object was an ObjectValue, a new object was created using Object.create(null), and then the properties were copied to this object.

Sure enough Object.create(null) creates an object without a prototype, which can lead to some interesting behavior, as documented here.

This PR changes this use of Object.create(null) to {}, giving the coerced object a full object prototype, and good behavior as it's being used elsewhere in my application.

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Ted Howard (d2a762d)

[IvanGoncharov]

Hi @tedchoward
Thanks for taking the time to open a PR and write such a detailed description 👍
This is a known issue, we knowingly use Object.create(null) see this explanation: #504 (comment)

In 16.0.0 we switch to TS but in 17.0.0 we will focus on implementing proper solution (switching to ES6 Map) as described in #2664

[tedchoward]

Thanks for the reply, and I apologize that this was a duplicate issue.

[glasser]

Something that's a bit odd is that valueFromAST uses Object.create(null) to set up input objects when the input object is a literal in the document, whereas coerceInputValue uses {} when the input object is coming from a variable value. This has surprised some Apollo Server users; see apollographql/apollo-server#3803 (comment)