In the name of performance, I think this is a sensible default and a direction we should move toward but this seems to be more of a breaking change than we could include in anything less than a major release.

While Apollo Server doesn't currently use the loc information from the AST, we currently pass the schema to plugins via the serverWillStart life-cycle event:

While this plugin API is its infancy, it's hard to be certain if existing plugins have in any way relied on loc properties which may now become non-existent.

More pressingly though, we use the schema to generate a schemaHash, used for various aspects of Apollo Engine and also passed into plugin life-cycle hooks, based on the introspection result:

Surprisingly, the result of the introspection seems to actually be different — in a not clear way — when the loc is not included (as a result of the new default), as this rudimentary script is showing for me:

// Install these dependencies!
// > npm install graphql graphql-tools deep-diff fast-json-stable-stringify

const assert = require('assert');
const { parse, execute, getIntrospectionQuery } = require('graphql');
const { makeExecutableSchema } = require('graphql-tools');
const diff = require('deep-diff').diff;
const stableStringify = require('fast-json-stable-stringify');
const { createHash } = require('crypto');

// Schema
typeDefs = 'type Query { myField: String }';
resolvers = {
  Query: {
    myField() {
      return "ok";
    }
  }
};

// Prepare the introspection query to run against the two schemas.
const introspection = parse(getIntrospectionQuery());

// Make one schema without the `noLocation` setting and one with it.
schema1 = makeExecutableSchema({
  typeDefs,
  resolvers,
  parseOptions: {},
});
schema2 = makeExecutableSchema({
  typeDefs,
  resolvers,
  parseOptions: { noLocation: true },
});

// Execute the introspectionQuery against them both.
schema1Result = execute(schema1, introspection).data.__schema
schema2Result = execute(schema2, introspection).data.__schema

// Make sure the two are the same.
assert.equal(
  changes = diff(schema1Result, schema2Result),
  undefined,
  "There were differences in the introspection schema: " + util.inspect(changes)
);

For me, the result of this is:

AssertionError [ERR_ASSERTION]: There were differences in the introspection schema: [ DiffEdit {
    kind: 'E',
    path: [ 'types', 0, 'description' ],
    lhs: '',
    rhs: null },
  DiffEdit {
    kind: 'E',
    path: [ 'types', 0, 'fields', 0, 'description' ],
    lhs: '',
    rhs: null } ]

The output here is showing that the types[0].description property and the types[0].fields[0].description property have changed from a blank string to null.

I'm slightly shocked by this difference, but this would be problematic in a patch release since I believe it will require manual intervention to republish the schema to Engine, etc.
(Again, not to mention what plugins might be doing with schemaHash or the loc properties.)

Since the schema is something which can be passed into the ApolloServer constructor, one short-term option is to have implementors who wish to have that memory reduction call makeExecutableSchema themselves, taking care to pass in the appropriate noLocation setting in its parseOptions. I'm not sure if we can do this as a non-breaking change in any other way at the moment, though really, I don't understand why those description properties should be be changing (I think that needs a bit more investigation) and we have no way of knowing if anyone actually uses those

LGTM!

I do think we should make this a default in the future!