Skip to content

Commit

Permalink
Merge branch 'nullable_change_warning' of https://github.com/aolesky/…
Browse files Browse the repository at this point in the history 
…graphql-js into aolesky-nullable_change_warning
  • Loading branch information
@leebyron
leebyron committed Dec 1, 2017
2 parents a3afb98 + c167f26 commit db4cfdc
Show file tree
Hide file tree
Showing 2 changed files with 173 additions and 47 deletions.
124 changes: 120 additions & 4 deletions src/utilities/__tests__/findBreakingChanges-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ import {
DangerousChangeType,
findBreakingChanges,
findDangerousChanges,
findFieldsThatChangedType,
findFieldsThatChangedTypeOnObjectOrInterfaceTypes,
findFieldsThatChangedTypeOnInputObjectTypes,
findRemovedTypes,
findTypesRemovedFromUnions,
findTypesAddedToUnions,
Expand Down Expand Up @@ -260,7 +261,10 @@ describe('findBreakingChanges', () => {
description: 'Type1.field18 changed type from [[Int!]!] to [[Int!]].',
},
];
expect(findFieldsThatChangedType(oldSchema, newSchema)).to.eql(
expect(findFieldsThatChangedTypeOnObjectOrInterfaceTypes(
oldSchema,
newSchema
)).to.eql(
expectedFieldChanges
);
});
Expand Down Expand Up @@ -437,7 +441,10 @@ describe('findBreakingChanges', () => {
'[[Int!]!].',
},
];
expect(findFieldsThatChangedType(oldSchema, newSchema)).to.eql(
expect(findFieldsThatChangedTypeOnInputObjectTypes(
oldSchema,
newSchema
).breakingChanges).to.eql(
expectedFieldChanges,
);
}
Expand Down Expand Up @@ -487,7 +494,10 @@ describe('findBreakingChanges', () => {
'InputType1 was added.',
},
];
expect(findFieldsThatChangedType(oldSchema, newSchema)).to.eql(
expect(findFieldsThatChangedTypeOnInputObjectTypes(
oldSchema,
newSchema
).breakingChanges).to.eql(
expectedFieldChanges,
);
});
Expand Down Expand Up @@ -1526,6 +1536,56 @@ describe('findDangerousChanges', () => {
]);
});

it('should detect if a nullable field was added to an input', () => {
const oldInputType = new GraphQLInputObjectType({
name: 'InputType1',
fields: {
field1: {
type: GraphQLString,
},
},
});

const newInputType = new GraphQLInputObjectType({
name: 'InputType1',
fields: {
field1: {
type: GraphQLString,
},
field2: {
type: GraphQLInt,
},
},
});

const oldSchema = new GraphQLSchema({
query: queryType,
types: [
oldInputType,
]
});

const newSchema = new GraphQLSchema({
query: queryType,
types: [
newInputType,
]
});

const expectedFieldChanges = [
{
type: DangerousChangeType.NULLABLE_INPUT_FIELD_ADDED,
description: 'A nullable field field2 on input type ' +
'InputType1 was added.',
},
];

expect(findFieldsThatChangedTypeOnInputObjectTypes(
oldSchema,
newSchema
).dangerousChanges).to.eql(expectedFieldChanges);
});

it('should find all dangerous changes', () => {
const enumThatGainsAValueOld = new GraphQLEnumType({
name: 'EnumType1',
Expand Down Expand Up @@ -1668,4 +1728,60 @@ describe('findDangerousChanges', () => {
expectedDangerousChanges
);
});

it('should detect if a nullable field argument was added', () => {
const oldType = new GraphQLObjectType({
name: 'Type1',
fields: {
field1: {
type: GraphQLString,
args: {
arg1: {
type: GraphQLString,
},
},
},
},
});

const newType = new GraphQLObjectType({
name: 'Type1',
fields: {
field1: {
type: GraphQLString,
args: {
arg1: {
type: GraphQLString,
},
arg2: {
type: GraphQLString,
},
},
},
},
});

const oldSchema = new GraphQLSchema({
query: queryType,
types: [
oldType,
]
});

const newSchema = new GraphQLSchema({
query: queryType,
types: [
newType,
]
});

expect(
findArgChanges(oldSchema, newSchema).dangerousChanges
).to.eql([
{
type: DangerousChangeType.NULLABLE_ARG_ADDED,
description: 'A nullable arg arg2 on Type1.field1 was added',
},
]);
});
});
96 changes: 53 additions & 43 deletions src/utilities/findBreakingChanges.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ export const DangerousChangeType = {
VALUE_ADDED_TO_ENUM: 'VALUE_ADDED_TO_ENUM',
INTERFACE_ADDED_TO_OBJECT: 'INTERFACE_ADDED_TO_OBJECT',
TYPE_ADDED_TO_UNION: 'TYPE_ADDED_TO_UNION',
NULLABLE_INPUT_FIELD_ADDED: 'NULLABLE_INPUT_FIELD_ADDED',
NULLABLE_ARG_ADDED: 'NULLABLE_ARG_ADDED',
};

export type BreakingChange = {
Expand All @@ -69,7 +71,9 @@ export function findBreakingChanges(
return [
...findRemovedTypes(oldSchema, newSchema),
...findTypesThatChangedKind(oldSchema, newSchema),
...findFieldsThatChangedType(oldSchema, newSchema),
...findFieldsThatChangedTypeOnObjectOrInterfaceTypes(oldSchema, newSchema),
...findFieldsThatChangedTypeOnInputObjectTypes(oldSchema, newSchema)
.breakingChanges,
...findTypesRemovedFromUnions(oldSchema, newSchema),
...findValuesRemovedFromEnums(oldSchema, newSchema),
...findArgChanges(oldSchema, newSchema).breakingChanges,
Expand All @@ -90,6 +94,8 @@ export function findDangerousChanges(
...findValuesAddedToEnums(oldSchema, newSchema),
...findInterfacesAddedToObjectTypes(oldSchema, newSchema),
...findTypesAddedToUnions(oldSchema, newSchema),
...findFieldsThatChangedTypeOnInputObjectTypes(oldSchema, newSchema)
.dangerousChanges
];
}

Expand Down Expand Up @@ -224,12 +230,20 @@ export function findArgChanges(
const oldArgDef = oldArgs.find(
arg => arg.name === newArgDef.name
);
if (!oldArgDef && newArgDef.type instanceof GraphQLNonNull) {
breakingChanges.push({
type: BreakingChangeType.NON_NULL_ARG_ADDED,
description: `A non-null arg ${newArgDef.name} on ` +
`${newType.name}.${fieldName} was added`,
});
if (!oldArgDef) {
if (newArgDef.type instanceof GraphQLNonNull) {
breakingChanges.push({
type: BreakingChangeType.NON_NULL_ARG_ADDED,
description: `A non-null arg ${newArgDef.name} on ` +
`${newType.name}.${fieldName} was added`,
});
} else {
dangerousChanges.push({
type: DangerousChangeType.NULLABLE_ARG_ADDED,
description: `A nullable arg ${newArgDef.name} on ` +
`${newType.name}.${fieldName} was added`,
});
}
}
});
});
Expand Down Expand Up @@ -263,30 +277,14 @@ function typeKindName(type: GraphQLNamedType): string {
throw new TypeError('Unknown type ' + type.constructor.name);
}

/**
* Given two schemas, returns an Array containing descriptions of any breaking
* changes in the newSchema related to the fields on a type. This includes if
* a field has been removed from a type, if a field has changed type, or if
* a non-null field is added to an input type.
*/
export function findFieldsThatChangedType(
oldSchema: GraphQLSchema,
newSchema: GraphQLSchema
): Array<BreakingChange> {
return [
...findFieldsThatChangedTypeOnObjectOrInterfaceTypes(oldSchema, newSchema),
...findFieldsThatChangedTypeOnInputObjectTypes(oldSchema, newSchema),
];
}

function findFieldsThatChangedTypeOnObjectOrInterfaceTypes(
export function findFieldsThatChangedTypeOnObjectOrInterfaceTypes(
oldSchema: GraphQLSchema,
newSchema: GraphQLSchema,
): Array<BreakingChange> {
const oldTypeMap = oldSchema.getTypeMap();
const newTypeMap = newSchema.getTypeMap();

const breakingFieldChanges = [];
const breakingChanges = [];
Object.keys(oldTypeMap).forEach(typeName => {
const oldType = oldTypeMap[typeName];
const newType = newTypeMap[typeName];
Expand All @@ -303,7 +301,7 @@ function findFieldsThatChangedTypeOnObjectOrInterfaceTypes(
Object.keys(oldTypeFieldsDef).forEach(fieldName => {
// Check if the field is missing on the type in the new schema.
if (!(fieldName in newTypeFieldsDef)) {
breakingFieldChanges.push({
breakingChanges.push({
type: BreakingChangeType.FIELD_REMOVED,
description: `${typeName}.${fieldName} was removed.`,
});
Expand All @@ -319,7 +317,7 @@ function findFieldsThatChangedTypeOnObjectOrInterfaceTypes(
const newFieldTypeString = isNamedType(newFieldType) ?
newFieldType.name :
newFieldType.toString();
breakingFieldChanges.push({
breakingChanges.push({
type: BreakingChangeType.FIELD_CHANGED_KIND,
description: `${typeName}.${fieldName} changed type from ` +
`${oldFieldTypeString} to ${newFieldTypeString}.`,
Expand All @@ -328,17 +326,21 @@ function findFieldsThatChangedTypeOnObjectOrInterfaceTypes(
}
});
});
return breakingFieldChanges;
return breakingChanges;
}

export function findFieldsThatChangedTypeOnInputObjectTypes(
oldSchema: GraphQLSchema,
newSchema: GraphQLSchema
): Array<BreakingChange> {
): {
breakingChanges: Array<BreakingChange>,
dangerousChanges: Array<DangerousChange>
} {
const oldTypeMap = oldSchema.getTypeMap();
const newTypeMap = newSchema.getTypeMap();

const breakingFieldChanges = [];
const breakingChanges = [];
const dangerousChanges = [];
Object.keys(oldTypeMap).forEach(typeName => {
const oldType = oldTypeMap[typeName];
const newType = newTypeMap[typeName];
Expand All @@ -354,7 +356,7 @@ export function findFieldsThatChangedTypeOnInputObjectTypes(
Object.keys(oldTypeFieldsDef).forEach(fieldName => {
// Check if the field is missing on the type in the new schema.
if (!(fieldName in newTypeFieldsDef)) {
breakingFieldChanges.push({
breakingChanges.push({
type: BreakingChangeType.FIELD_REMOVED,
description: `${typeName}.${fieldName} was removed.`,
});
Expand All @@ -371,29 +373,37 @@ export function findFieldsThatChangedTypeOnInputObjectTypes(
const newFieldTypeString = isNamedType(newFieldType) ?
newFieldType.name :
newFieldType.toString();
breakingFieldChanges.push({
breakingChanges.push({
type: BreakingChangeType.FIELD_CHANGED_KIND,
description: `${typeName}.${fieldName} changed type from ` +
`${oldFieldTypeString} to ${newFieldTypeString}.`,
});
}
}
});
// Check if a non-null field was added to the input object type
// Check if a field was added to the input object type
Object.keys(newTypeFieldsDef).forEach(fieldName => {
if (
!(fieldName in oldTypeFieldsDef) &&
newTypeFieldsDef[fieldName].type instanceof GraphQLNonNull
) {
breakingFieldChanges.push({
type: BreakingChangeType.NON_NULL_INPUT_FIELD_ADDED,
description: `A non-null field ${fieldName} on ` +
`input type ${newType.name} was added.`,
});
if (!(fieldName in oldTypeFieldsDef)) {
if (newTypeFieldsDef[fieldName].type instanceof GraphQLNonNull) {
breakingChanges.push({
type: BreakingChangeType.NON_NULL_INPUT_FIELD_ADDED,
description: `A non-null field ${fieldName} on ` +
`input type ${newType.name} was added.`,
});
} else {
dangerousChanges.push({
type: DangerousChangeType.NULLABLE_INPUT_FIELD_ADDED,
description: `A nullable field ${fieldName} on ` +
`input type ${newType.name} was added.`,
});
}
}
});
});
return breakingFieldChanges;
return {
breakingChanges,
dangerousChanges,
};
}

function isChangeSafeForObjectOrInterfaceField(
Expand Down

0 comments on commit db4cfdc

Please sign in to comment.