Skip to content

Commit

Permalink
Refactor for better typing
Browse files Browse the repository at this point in the history 
Strongly type the return objects, use invariants in a way to satisfy
flow, and DRY up one common function.
  • Loading branch information
@eapache
eapache committed Jan 2, 2018
1 parent ae3ba3a commit 4576e13
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 81 deletions.
12 changes: 6 additions & 6 deletions src/utilities/__tests__/findDescriptionChanges-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ describe('findDescriptionChanges', () => {
query: queryType,
types: [typeNew],
});
expect(findDescriptionChanges(oldSchema, newSchema)).to.eql([
'Description added on type Type.',
]);
expect(findDescriptionChanges(oldSchema, newSchema)[0].description).to.eql(
'Description added on TYPE Type.',
);
expect(findDescriptionChanges(oldSchema, oldSchema)).to.eql([]);
expect(findDescriptionChanges(newSchema, newSchema)).to.eql([]);
});
Expand All @@ -67,9 +67,9 @@ describe('findDescriptionChanges', () => {
query: queryType,
types: [type],
});
expect(findDescriptionChanges(oldSchema, newSchema)).to.eql([
'Description added on new type Type.',
]);
expect(findDescriptionChanges(oldSchema, newSchema)[0].description).to.eql(
'New TYPE Type added with description.',
);
expect(findDescriptionChanges(oldSchema, oldSchema)).to.eql([]);
expect(findDescriptionChanges(newSchema, newSchema)).to.eql([]);
});
Expand Down
171 changes: 96 additions & 75 deletions src/utilities/findDescriptionChanges.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,28 @@ import {
} from '../type/definition';
import type { GraphQLFieldMap } from '../type/definition';
import { GraphQLSchema } from '../type/schema';
import invariant from '../jsutils/invariant';

export const DescribedObjectType = {
FIELD: 'FIELD',
TYPE: 'TYPE',
ARGUMENT: 'ARGUMENT',
ENUM_VALUE: 'ENUM_VALUE',
};

export const DescriptionChangeType = {
OBJECT_ADDED: 'OBJECT_ADDED',
DESCRIPTION_ADDED: 'DESCRIPTION_ADDED',
DESCRIPTION_CHANGED: 'DESCRIPTION_CHANGED',
};

export type DescriptionChange = {
object: $Keys<typeof DescribedObjectType>,
change: $Keys<typeof DescriptionChangeType>,
description: string,
oldThing: any,
newThing: any,
};

/**
* Given two schemas, returns an Array containing descriptions of any
Expand All @@ -17,37 +39,32 @@ import { GraphQLSchema } from '../type/schema';
export function findDescriptionChanges(
oldSchema: GraphQLSchema,
newSchema: GraphQLSchema,
): Array<string> {
): Array<DescriptionChange> {
const oldTypeMap = oldSchema.getTypeMap();
const newTypeMap = newSchema.getTypeMap();

const descriptionChanges: Array<string> = [];
const descriptionChanges: Array<?DescriptionChange> = [];

Object.keys(newTypeMap).forEach(typeName => {
const oldType = oldTypeMap[typeName];
const newType = newTypeMap[typeName];

if (newType.description) {
if (!oldType) {
descriptionChanges.push(
`Description added on new type ${newType.name}.`,
);
} else if (!oldType.description) {
descriptionChanges.push(`Description added on type ${newType.name}.`);
} else if (oldType.description !== newType.description) {
descriptionChanges.push(`Description changed on type ${newType.name}.`);
}
}

if (oldType && !(newType instanceof oldType.constructor)) {
return;
}
descriptionChanges.push(
generateDescriptionChange(newType, oldType, DescribedObjectType.TYPE),
);

if (
newType instanceof GraphQLObjectType ||
newType instanceof GraphQLInterfaceType ||
newType instanceof GraphQLInputObjectType
) {
invariant(
!oldType ||
oldType instanceof GraphQLObjectType ||
oldType instanceof GraphQLInterfaceType ||
oldType instanceof GraphQLInputObjectType,
'Expected oldType to also have fields',
);
const oldTypeFields: ?GraphQLFieldMap<*, *> = oldType
? oldType.getFields()
: null;
Expand All @@ -57,21 +74,13 @@ export function findDescriptionChanges(
const oldField = oldTypeFields ? oldTypeFields[fieldName] : null;
const newField = newTypeFields[fieldName];

if (newField.description) {
if (!oldField) {
descriptionChanges.push(
`Description added on new field ${newType.name}.${newField.name}`,
);
} else if (!oldField.description) {
descriptionChanges.push(
`Description added on field ${newType.name}.${newField.name}.`,
);
} else if (oldField.description !== newField.description) {
descriptionChanges.push(
`Description changed on field ${newType.name}.${newField.name}.`,
);
}
}
descriptionChanges.push(
generateDescriptionChange(
newField,
oldField,
DescribedObjectType.FIELD,
),
);

if (!newField.args) {
return;
Expand All @@ -82,61 +91,73 @@ export function findDescriptionChanges(
? oldField.args.find(arg => arg.name === newArg.name)
: null;

if (newArg.description) {
if (!oldArg) {
descriptionChanges.push(
`Description added on new arg ${newType.name}.${
newField.name
}.${newArg.name}.`,
);
} else if (!oldArg.description) {
descriptionChanges.push(
`Description added on arg ${newType.name}.${newField.name}.${
newArg.name
}.`,
);
} else if (oldArg.description !== newArg.description) {
descriptionChanges.push(
`Description changed on arg ${newType.name}.${newField.name}.${
newArg.name
}.`,
);
}
}
descriptionChanges.push(
generateDescriptionChange(
newArg,
oldArg,
DescribedObjectType.ARGUMENT,
),
);
});
});
} else if (newType instanceof GraphQLEnumType) {
invariant(
!oldType || oldType instanceof GraphQLEnumType,
'Expected oldType to also have values',
);
const oldValues = oldType ? oldType.getValues() : null;
const newValues = newType.getValues();
newValues.forEach(newValue => {
const oldValue = oldValues
? oldValues.find(value => value.name === newValue.name)
: null;

if (newValue.description) {
if (!oldValue) {
descriptionChanges.push(
`Description added on enum value ${newType.name}.${
newValue.name
}.`,
);
} else if (!oldValue.description) {
descriptionChanges.push(
`Description added on enum value ${newType.name}.${
newValue.name
}.`,
);
} else if (oldValue.description !== newValue.description) {
descriptionChanges.push(
`Description changed on enum value ${newType.name}.${
newValue.name
}.`,
);
}
}
descriptionChanges.push(
generateDescriptionChange(
newValue,
oldValue,
DescribedObjectType.ENUM_VALUE,
),
);
});
}
});

return descriptionChanges;
return descriptionChanges.filter(Boolean);
}

function generateDescriptionChange(
newThing,
oldThing,
objectType: $Keys<typeof DescribedObjectType>,
): ?DescriptionChange {
if (!newThing.description) {
return;
}

if (!oldThing) {
return {
object: objectType,
change: DescriptionChangeType.OBJECT_ADDED,
oldThing,
newThing,
description: `New ${objectType} ${newThing.name} added with description.`,
};
} else if (!oldThing.description) {
return {
object: objectType,
change: DescriptionChangeType.DESCRIPTION_ADDED,
oldThing,
newThing,
description: `Description added on ${objectType} ${newThing.name}.`,
};
} else if (oldThing.description !== newThing.description) {
return {
object: objectType,
change: DescriptionChangeType.DESCRIPTION_CHANGED,
oldThing,
newThing,
description: `Description changed on ${objectType} ${newThing.name}.`,
};
}
}

0 comments on commit 4576e13

Please sign in to comment.