From 67162e5f2aeb642c436a21a664fe57b0220ede3f Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Fri, 13 Jul 2018 19:52:06 +0300 Subject: [PATCH] Fix memory leak in buildSchema/extendSchema --- .eslintrc | 2 +- src/type/definition.js | 140 +++++++++++++++++++++-------------------- 2 files changed, 72 insertions(+), 70 deletions(-) diff --git a/.eslintrc b/.eslintrc index f0988229eb..c65d4a32c5 100644 --- a/.eslintrc +++ b/.eslintrc @@ -42,7 +42,7 @@ "rules": { "prettier/prettier": 2, - "flowtype/space-after-type-colon": [2, "always"], + "flowtype/space-after-type-colon": 0, "flowtype/space-before-type-colon": [2, "never"], "flowtype/space-before-generic-bracket": [2, "never"], "flowtype/union-intersection-spacing": [2, "always"], diff --git a/src/type/definition.js b/src/type/definition.js index b74dcde582..fd288f3c42 100644 --- a/src/type/definition.js +++ b/src/type/definition.js @@ -649,9 +649,8 @@ export class GraphQLObjectType { extensionASTNodes: ?$ReadOnlyArray; isTypeOf: ?GraphQLIsTypeOfFn<*, *>; - _typeConfig: GraphQLObjectTypeConfig<*, *>; - _fields: GraphQLFieldMap<*, *>; - _interfaces: Array; + _fields: Thunk>; + _interfaces: Thunk>; constructor(config: GraphQLObjectTypeConfig<*, *>): void { this.name = config.name; @@ -659,7 +658,8 @@ export class GraphQLObjectType { this.astNode = config.astNode; this.extensionASTNodes = config.extensionASTNodes; this.isTypeOf = config.isTypeOf; - this._typeConfig = config; + this._fields = defineFieldMap.bind(undefined, config); + this._interfaces = defineInterfaces.bind(undefined, config); invariant(typeof config.name === 'string', 'Must provide name.'); if (config.isTypeOf) { invariant( @@ -670,17 +670,17 @@ export class GraphQLObjectType { } getFields(): GraphQLFieldMap<*, *> { - return ( - this._fields || - (this._fields = defineFieldMap(this, this._typeConfig.fields)) - ); + if (typeof this._fields === 'function') { + this._fields = this._fields(); + } + return this._fields; } getInterfaces(): Array { - return ( - this._interfaces || - (this._interfaces = defineInterfaces(this, this._typeConfig.interfaces)) - ); + if (typeof this._interfaces === 'function') { + this._interfaces = this._interfaces(); + } + return this._interfaces; } toString(): string { @@ -693,26 +693,26 @@ defineToStringTag(GraphQLObjectType); defineToJSON(GraphQLObjectType); function defineInterfaces( - type: GraphQLObjectType, - interfacesThunk: Thunk>, + config: GraphQLObjectTypeConfig<*, *>, ): Array { - const interfaces = resolveThunk(interfacesThunk) || []; + const interfaces = resolveThunk(config.interfaces) || []; invariant( Array.isArray(interfaces), - `${type.name} interfaces must be an Array or a function which returns ` + + `${config.name} interfaces must be an Array or a function which returns ` + 'an Array.', ); return interfaces; } function defineFieldMap( - type: GraphQLNamedType, - fieldsThunk: Thunk>, + config: + | GraphQLObjectTypeConfig + | GraphQLInterfaceTypeConfig, ): GraphQLFieldMap { - const fieldMap = resolveThunk(fieldsThunk) || {}; + const fieldMap = resolveThunk(config.fields) || {}; invariant( isPlainObj(fieldMap), - `${type.name} fields must be an object with field names as keys or a ` + + `${config.name} fields must be an object with field names as keys or a ` + 'function which returns such an object.', ); @@ -721,12 +721,12 @@ function defineFieldMap( const fieldConfig = fieldMap[fieldName]; invariant( isPlainObj(fieldConfig), - `${type.name}.${fieldName} field config must be an object`, + `${config.name}.${fieldName} field config must be an object`, ); invariant( !fieldConfig.hasOwnProperty('isDeprecated'), - `${type.name}.${fieldName} should provide "deprecationReason" instead ` + - 'of "isDeprecated".', + `${config.name}.${fieldName} should provide "deprecationReason" ` + + 'instead of "isDeprecated".', ); const field = { ...fieldConfig, @@ -735,7 +735,7 @@ function defineFieldMap( }; invariant( isValidResolver(field.resolve), - `${type.name}.${fieldName} field resolver must be a function if ` + + `${config.name}.${fieldName} field resolver must be a function if ` + `provided, but got: ${inspect(field.resolve)}.`, ); const argsConfig = fieldConfig.args; @@ -744,7 +744,7 @@ function defineFieldMap( } else { invariant( isPlainObj(argsConfig), - `${type.name}.${fieldName} args must be an object with argument ` + + `${config.name}.${fieldName} args must be an object with argument ` + 'names as keys.', ); field.args = Object.keys(argsConfig).map(argName => { @@ -903,8 +903,7 @@ export class GraphQLInterfaceType { extensionASTNodes: ?$ReadOnlyArray; resolveType: ?GraphQLTypeResolver<*, *>; - _typeConfig: GraphQLInterfaceTypeConfig<*, *>; - _fields: GraphQLFieldMap<*, *>; + _fields: Thunk>; constructor(config: GraphQLInterfaceTypeConfig<*, *>): void { this.name = config.name; @@ -912,7 +911,7 @@ export class GraphQLInterfaceType { this.astNode = config.astNode; this.extensionASTNodes = config.extensionASTNodes; this.resolveType = config.resolveType; - this._typeConfig = config; + this._fields = defineFieldMap.bind(undefined, config); invariant(typeof config.name === 'string', 'Must provide name.'); if (config.resolveType) { invariant( @@ -923,10 +922,10 @@ export class GraphQLInterfaceType { } getFields(): GraphQLFieldMap<*, *> { - return ( - this._fields || - (this._fields = defineFieldMap(this, this._typeConfig.fields)) - ); + if (typeof this._fields === 'function') { + this._fields = this._fields(); + } + return this._fields; } toString(): string { @@ -982,8 +981,7 @@ export class GraphQLUnionType { extensionASTNodes: ?$ReadOnlyArray; resolveType: ?GraphQLTypeResolver<*, *>; - _typeConfig: GraphQLUnionTypeConfig<*, *>; - _types: Array; + _types: Thunk>; constructor(config: GraphQLUnionTypeConfig<*, *>): void { this.name = config.name; @@ -991,7 +989,7 @@ export class GraphQLUnionType { this.astNode = config.astNode; this.extensionASTNodes = config.extensionASTNodes; this.resolveType = config.resolveType; - this._typeConfig = config; + this._types = defineTypes.bind(undefined, config); invariant(typeof config.name === 'string', 'Must provide name.'); if (config.resolveType) { invariant( @@ -1002,9 +1000,10 @@ export class GraphQLUnionType { } getTypes(): Array { - return ( - this._types || (this._types = defineTypes(this, this._typeConfig.types)) - ); + if (typeof this._types === 'function') { + this._types = this._types(); + } + return this._types; } toString(): string { @@ -1017,14 +1016,13 @@ defineToStringTag(GraphQLUnionType); defineToJSON(GraphQLUnionType); function defineTypes( - unionType: GraphQLUnionType, - typesThunk: Thunk>, + config: GraphQLUnionTypeConfig<*, *>, ): Array { - const types = resolveThunk(typesThunk) || []; + const types = resolveThunk(config.types) || []; invariant( Array.isArray(types), 'Must provide Array of types or a function which returns ' + - `such an array for Union ${unionType.name}.`, + `such an array for Union ${config.name}.`, ); return types; } @@ -1216,43 +1214,22 @@ export class GraphQLInputObjectType { astNode: ?InputObjectTypeDefinitionNode; extensionASTNodes: ?$ReadOnlyArray; - _typeConfig: GraphQLInputObjectTypeConfig; - _fields: GraphQLInputFieldMap; + _fields: Thunk; constructor(config: GraphQLInputObjectTypeConfig): void { this.name = config.name; this.description = config.description; this.astNode = config.astNode; this.extensionASTNodes = config.extensionASTNodes; - this._typeConfig = config; + this._fields = defineInputFieldMap.bind(undefined, config); invariant(typeof config.name === 'string', 'Must provide name.'); } getFields(): GraphQLInputFieldMap { - return this._fields || (this._fields = this._defineFieldMap()); - } - - _defineFieldMap(): GraphQLInputFieldMap { - const fieldMap: any = resolveThunk(this._typeConfig.fields) || {}; - invariant( - isPlainObj(fieldMap), - `${this.name} fields must be an object with field names as keys or a ` + - 'function which returns such an object.', - ); - const resultFieldMap = Object.create(null); - Object.keys(fieldMap).forEach(fieldName => { - const field = { - ...fieldMap[fieldName], - name: fieldName, - }; - invariant( - !field.hasOwnProperty('resolve'), - `${this.name}.${fieldName} field type has a resolve property, but ` + - 'Input Types cannot define resolvers.', - ); - resultFieldMap[fieldName] = field; - }); - return resultFieldMap; + if (typeof this._fields === 'function') { + this._fields = this._fields(); + } + return this._fields; } toString(): string { @@ -1264,6 +1241,31 @@ export class GraphQLInputObjectType { defineToStringTag(GraphQLInputObjectType); defineToJSON(GraphQLInputObjectType); +function defineInputFieldMap( + config: GraphQLInputObjectTypeConfig, +): GraphQLInputFieldMap { + const fieldMap: any = resolveThunk(config.fields) || {}; + invariant( + isPlainObj(fieldMap), + `${config.name} fields must be an object with field names as keys or a ` + + 'function which returns such an object.', + ); + const resultFieldMap = Object.create(null); + Object.keys(fieldMap).forEach(fieldName => { + const field = { + ...fieldMap[fieldName], + name: fieldName, + }; + invariant( + !field.hasOwnProperty('resolve'), + `${config.name}.${fieldName} field type has a resolve property, but ` + + 'Input Types cannot define resolvers.', + ); + resultFieldMap[fieldName] = field; + }); + return resultFieldMap; +} + export type GraphQLInputObjectTypeConfig = {| name: string, fields: Thunk,