Allow passing parseOptions to ApolloServerBase constructor.

[benjamn]

Note: the first commit in this PR made { noLocation: true } the default parsing behavior, but that behavior has been reverted because it could be a breaking change.

Memory-wise, the server.schema object is often the heaviest component of an ApolloServerBase object, especially since schemas can be arbitrarily large.

Although much of the information contained by the schema is important, by default the graphql/language/parser implementation attaches a .loc property to every AST node, which contains information about the starting/ending source locations of the node, as well as a linked list of all the token objects contained by the node, and a reference to the source string.

Since Apollo Server does not depend on this location/token/source information, we can save a considerable amount of memory by discarding loc properties, as suggested here:

export type ParseOptions = {
  /**
   * By default, the parser creates AST nodes that know the location
   * in the source that they correspond to. This configuration flag
   * disables that behavior for performance or testing.
   */
  noLocation?: boolean,

Here's a screenshot from a memory profiler showing allocations retained by a GraphQLSchema object in a typical React Native application (the 289KB @14998 object is the schema):

After enabling the { noLocation: true } parse option, the retained size of the schema drops from 289.03KB to 178.65KB (a 38% improvement), and you can see the loc fields are no longer present:

Note: the objects higher in this list either contain the schema object (e.g. the ApolloServerBase object), or represent the module system itself (the first/largest item), which contains virtually everything in a React Native application.

Consumers who need location/token information in their schema can either create their own schema object and pass it into the ApolloServer constructor, or pass in custom parseOptions as an option to control the behavior of makeExecutableSchema.

[abernix]

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:

apollo-server/packages/apollo-server-core/src/ApolloServer.ts

Line 112 in c5b71f8

protected schema: GraphQLSchema;

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:

apollo-server/packages/apollo-server-core/src/ApolloServer.ts

Line 312 in c5b71f8

this.schemaHash = generateSchemaHash(this.schema);

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

[benjamn]

@abernix Thanks for the feedback! Have another look at the most recent changes?

[abernix]

LGTM!

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

[abernix]

My 👀s and 🧠 are happy you've de-composed this.