Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further improve infer fields recursion #2452

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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