Skip to content

Commit

Permalink
Further improve infer fields recursion (#2452)
Browse files Browse the repository at this point in the history
* add another test for type inference recursion, convert checking to use Set()

* also test type inference recursion for objects with no further keys, and ID scalar

* handle ID scalars in type inference, and ensure no fieldless object types
  • Loading branch information
jasonphillips authored and KyleAMathews committed Oct 13, 2017
1 parent 6e00ddf commit 74f49fe
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const {
GraphQLBoolean,
GraphQLFloat,
GraphQLInt,
GraphQLID,
GraphQLNonNull,
GraphQLString,
GraphQLObjectType,
Expand All @@ -23,6 +24,14 @@ function isIntInput(type) {
})
}

function isIdInput(type) {
expect(type instanceof GraphQLInputObjectType).toBeTruthy()
expect(type.getFields()).toEqual({
eq: { name: `eq`, type: GraphQLID },
ne: { name: `ne`, type: GraphQLID },
})
}

function isStringInput(type) {
expect(type instanceof GraphQLInputObjectType).toBeTruthy()
expect(type.getFields()).toEqual({
Expand Down Expand Up @@ -133,12 +142,42 @@ describe(`GraphQL Input args from fields, test-only`, () => {
isStringInput(innerObjFields.foo.type)
})

it(`handles lists within lists`, async () => {
const Row = new GraphQLObjectType({
name: `Row`,
fields: () => {
return {
cells: typeField(new GraphQLList(Cell)),
}
},
})

const Cell = new GraphQLObjectType({
name: `Cell`,
fields: () => {
return {
value: typeField(GraphQLInt),
}
},
})

const fields = {
rows: typeField(new GraphQLList(Row)),
}

expect(() => {
inferInputObjectStructureFromFields({
fields,
typeName: `ListTypes`,
})
}).not.toThrow()
})

it(`protects against infinite recursion on circular definitions`, async () => {
const TypeA = new GraphQLObjectType({
name: `TypeA`,
fields: () => {
return {
foo: typeField(GraphQLInt),
typeb: typeField(TypeB),
}
},
Expand All @@ -148,7 +187,7 @@ describe(`GraphQL Input args from fields, test-only`, () => {
name: `TypeB`,
fields: () => {
return {
bar: typeField(GraphQLInt),
bar: typeField(GraphQLID),
typea: typeField(TypeA),
}
},
Expand All @@ -175,17 +214,20 @@ describe(`GraphQL Input args from fields, test-only`, () => {

expect(entryPointA instanceof GraphQLInputObjectType).toBeTruthy()
expect(entryPointB instanceof GraphQLInputObjectType).toBeTruthy()
isIntInput(entryPointAFields.foo.type)
isIntInput(entryPointBFields.bar.type)
isIdInput(entryPointBFields.bar.type)

// next level should also work, ie. typeA -> type B
const childAB = entryPointAFields.typeb.type
const childABFields = childAB.getFields()
expect(childAB instanceof GraphQLInputObjectType).toBeTruthy()
isIntInput(childABFields.bar.type)
isIdInput(childABFields.bar.type)

// circular level should not be here, ie. typeA -> typeB -> typeA
expect(childABFields.typea).toBeUndefined()

// in the other direction, from entryPointB -> typeA, the latter shouldn't exist,
// due to having no further non-circular fields to filter
expect(entryPointBFields.typea).toBeUndefined()
})

it(`recovers from unknown output types`, async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const {
GraphQLString,
GraphQLFloat,
GraphQLInt,
GraphQLID,
GraphQLList,
GraphQLEnumType,
GraphQLNonNull,
Expand Down Expand Up @@ -37,23 +38,27 @@ function makeNullable(type: GraphQLInputType): GraphQLNullableInputType<any> {

function convertToInputType(
type: GraphQLType,
typeMap: any
typeMap: Set
): ?GraphQLInputType {
// track types already processed in current tree, to avoid infinite recursion
if (typeMap[type.name]) {
if (typeMap.has(type)) {
return null
}
const nextTypeMap = { ...typeMap, [type.name]: true }
const nextTypeMap = new Set([...typeMap, type])

if (type instanceof GraphQLScalarType || type instanceof GraphQLEnumType) {
return type
} else if (type instanceof GraphQLObjectType) {
const fields = _.transform(type.getFields(), (out, fieldConfig, key) => {
const type = convertToInputType(fieldConfig.type, nextTypeMap)
if (type) out[key] = { type }
})
if (Object.keys(fields).length===0) {
return null
}
return new GraphQLInputObjectType({
name: createTypeName(`${type.name}InputObject`),
fields: _.transform(type.getFields(), (out, fieldConfig, key) => {
const type = convertToInputType(fieldConfig.type, nextTypeMap)
if (type) out[key] = { type }
}),
fields,
})
} else if (type instanceof GraphQLList) {
let innerType = convertToInputType(type.ofType, nextTypeMap)
Expand Down Expand Up @@ -85,6 +90,10 @@ const scalarFilterMap = {
eq: { type: GraphQLFloat },
ne: { type: GraphQLFloat },
},
ID: {
eq: { type: GraphQLID },
ne: { type: GraphQLID },
},
String: {
eq: { type: GraphQLString },
ne: { type: GraphQLString },
Expand Down Expand Up @@ -169,7 +178,7 @@ export function inferInputObjectStructureFromFields({
const sort = []

_.each(fields, (fieldConfig, key) => {
const inputType = convertToInputType(fieldConfig.type, {})
const inputType = convertToInputType(fieldConfig.type, new Set())
const inputFilter =
inputType && convertToInputFilter(_.upperFirst(key), inputType)

Expand Down

0 comments on commit 74f49fe

Please sign in to comment.