From d77f6a52efffc42f1eb8e486ba3209f5370ed23e Mon Sep 17 00:00:00 2001 From: Robert Zhu Date: Tue, 17 Jan 2017 18:26:00 -0500 Subject: [PATCH 1/2] Validate resolver definition is a function --- src/type/__tests__/validation-test.js | 43 +++++++++++++++++++++++++++ src/type/definition.js | 13 ++++++++ 2 files changed, 56 insertions(+) diff --git a/src/type/__tests__/validation-test.js b/src/type/__tests__/validation-test.js index e541ca7d2f..ac0471b962 100644 --- a/src/type/__tests__/validation-test.js +++ b/src/type/__tests__/validation-test.js @@ -1119,6 +1119,49 @@ describe('Type System: Object fields must have output types', () => { }); +describe('Type System: Object fields must have valid resolve values', () => { + + function schemaWithObjectWithFieldResolver(resolveValue) { + const BadResolverType = new GraphQLObjectType({ + name: 'BadResolver', + fields: { + badField: { + type: GraphQLString, + resolve: resolveValue + } + } + }); + + return new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + f: { type: BadResolverType } + } + }) + }); + } + + it('accepts a lambda as an Object field resolver', () => { + expect(() => schemaWithObjectWithFieldResolver(() => ({}))).not.to.throw(); + }); + + it('rejects an empty Object field resolver', () => { + expect(() => schemaWithObjectWithFieldResolver({})).to.throw( + 'BadResolver.badField field resolver must be function or null/undefined' + + ', but got: [object Object].' + ); + }); + + it('rejects a constant scalar value resolver', () => { + expect(() => schemaWithObjectWithFieldResolver(0)).to.throw( + 'BadResolver.badField field resolver must be function or null/undefined' + + ', but got: 0.' + ); + }); +}); + + describe('Type System: Objects can only implement interfaces', () => { function schemaWithObjectImplementingType(implementedType) { diff --git a/src/type/definition.js b/src/type/definition.js index 2cf5a61d86..22a0d37086 100644 --- a/src/type/definition.js +++ b/src/type/definition.js @@ -506,6 +506,11 @@ function defineFieldMap( `${type.name}.${fieldName} field type must be Output Type but ` + `got: ${String(field.type)}.` ); + invariant( + isValidResolver(field.resolve), + `${type.name}.${fieldName} field resolver must be function or null/` + + `undefined, but got: ${String(field.resolve)}.` + ); const argsConfig = fieldConfig.args; if (!argsConfig) { field.args = []; @@ -540,6 +545,14 @@ function isPlainObj(obj) { return obj && typeof obj === 'object' && !Array.isArray(obj); } +// If a resolver is defined, it must be a function. +function isValidResolver(resolver: any): boolean { + if (resolver === null || resolver === undefined) { + return true; + } + return typeof resolver === 'function'; +} + export type GraphQLObjectTypeConfig = { name: string; interfaces?: Thunk>; From 3ac1bc1a1d421eb481aa8a0f8def9dc6ef2f65d5 Mon Sep 17 00:00:00 2001 From: Robert Zhu Date: Thu, 19 Jan 2017 15:12:43 -0500 Subject: [PATCH 2/2] Update eslintrc to allow smart '== null' --- .eslintrc | 2 +- src/type/__tests__/validation-test.js | 8 ++++---- src/type/definition.js | 9 +++------ 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/.eslintrc b/.eslintrc index fc9bc6b3ce..e5e378f602 100644 --- a/.eslintrc +++ b/.eslintrc @@ -69,7 +69,7 @@ "dot-location": [2, "property"], "dot-notation": 0, "eol-last": 2, - "eqeqeq": 2, + "eqeqeq": ["error", "smart"], "func-names": 0, "func-style": 0, "generator-star-spacing": [2, {"before": true, "after": false}], diff --git a/src/type/__tests__/validation-test.js b/src/type/__tests__/validation-test.js index ac0471b962..1e321d0bba 100644 --- a/src/type/__tests__/validation-test.js +++ b/src/type/__tests__/validation-test.js @@ -1148,15 +1148,15 @@ describe('Type System: Object fields must have valid resolve values', () => { it('rejects an empty Object field resolver', () => { expect(() => schemaWithObjectWithFieldResolver({})).to.throw( - 'BadResolver.badField field resolver must be function or null/undefined' + - ', but got: [object Object].' + 'BadResolver.badField field resolver must be a function if provided, ' + + 'but got: [object Object].' ); }); it('rejects a constant scalar value resolver', () => { expect(() => schemaWithObjectWithFieldResolver(0)).to.throw( - 'BadResolver.badField field resolver must be function or null/undefined' + - ', but got: 0.' + 'BadResolver.badField field resolver must be a function if provided, ' + + 'but got: 0.' ); }); }); diff --git a/src/type/definition.js b/src/type/definition.js index 22a0d37086..541840cb6a 100644 --- a/src/type/definition.js +++ b/src/type/definition.js @@ -508,8 +508,8 @@ function defineFieldMap( ); invariant( isValidResolver(field.resolve), - `${type.name}.${fieldName} field resolver must be function or null/` + - `undefined, but got: ${String(field.resolve)}.` + `${type.name}.${fieldName} field resolver must be a function if ` + + `provided, but got: ${String(field.resolve)}.` ); const argsConfig = fieldConfig.args; if (!argsConfig) { @@ -547,10 +547,7 @@ function isPlainObj(obj) { // If a resolver is defined, it must be a function. function isValidResolver(resolver: any): boolean { - if (resolver === null || resolver === undefined) { - return true; - } - return typeof resolver === 'function'; + return (resolver == null || typeof resolver === 'function'); } export type GraphQLObjectTypeConfig = {