feat(objectid): add valueOf to ObjectId prototype

[sibelius]

fix #7299
fix graphql/graphql-js#1518

Summary

this make interop of ObjectId and GraphQL easier

Test plan

validate valueOf works as expected
check tests here #7299

[sibelius]

not sure if tests failing because of my changes

[vkarpov15]

Yeah the tests are failing because of your changes, I suspect because somewhere we check for .valueOf().

In 5.4.0 we introduced global getters for schema types, so you can do something like this:

const mongoose = require('mongoose');

// Do this instead
mongoose.ObjectId.get(v => v.toString());

mongoose.set('debug', true);
run().catch(error => console.error(error.stack));

async function run() {
  await mongoose.connect('mongodb://localhost:27017/test', { useNewUrlParser: true });
  const Model = mongoose.model('Test', new mongoose.Schema({ name: String }));
  const docs = await Model.create([{ name: 'test1' }, { name: 'test2' }]);

  console.log(typeof docs[0]._id);

  const doc = new Model(docs[0].toObject());
  console.log(docs[0]._id === doc._id); // true
}

See SchemaType.get(). I'm no expert on GraphQL, but the custom getter approach would be better on the Mongoose side because it doesn't pollute ObjectId.prototype.

I'll look into why the tests are failing in the meantime.

[vkarpov15]

I'm going to close this since I got alexmcmillan/graphql-mongodb-query-fail-demo#1 working without polluting ObjectId.prototype. Making this change is pretty risky - the test failure was due to one of our internal plugins checking for valueOf(). I imagine other plugins could run into similar issues.

Drop in this one-liner (requires Mongoose >= 5.4.0) and your GraphQL issues should go away:

mongoose.ObjectId.get(v => v == null ? v : v.toString());

See this for more info.