Skip to content

Commit

Permalink
addResolversToSchema should not lose directives (#1623)
Browse files Browse the repository at this point in the history 
* Bump @typescript-eslint/eslint-plugin from 3.1.0 to 3.2.0 (#1620)

* consolidate resolvers validation

* addResolversToSchema should merge scalar/enum resolvers

instead of overwriting

closes #1587

* perform shallow merge only for extensions

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
  • Loading branch information
@yaacovCR @dependabot
yaacovCR and dependabot[bot] committed Jun 10, 2020
1 parent a91db79 commit 5b93e0e
Show file tree
Hide file tree
Showing 2 changed files with 158 additions and 106 deletions.
235 changes: 129 additions & 106 deletions packages/schema/src/addResolversToSchema.ts
View file
Expand Up @@ -99,40 +99,60 @@ export function addResolversToSchema(
type[fieldName] = resolverValue[fieldName];
}
});
} else if (isEnumType(type)) {
const values = type.getValues();

Object.keys(resolverValue).forEach(fieldName => {
if (
!fieldName.startsWith('__') &&
!values.some(value => value.name === fieldName) &&
!allowResolversNotInSchema
) {
throw new Error(`${type.name}.${fieldName} was defined in resolvers, but not present within ${type.name}`);
}
});
} else if (isUnionType(type)) {
Object.keys(resolverValue).forEach(fieldName => {
if (!fieldName.startsWith('__') && !allowResolversNotInSchema) {
throw new Error(
`${type.name}.${fieldName} was defined in resolvers, but ${type.name} is not an object or interface type`
);
}
});
} else if (isObjectType(type) || isInterfaceType(type)) {
Object.keys(resolverValue).forEach(fieldName => {
if (!fieldName.startsWith('__')) {
const fields = type.getFields();
const field = fields[fieldName];

if (field == null && !allowResolversNotInSchema) {
throw new Error(`${typeName}.${fieldName} defined in resolvers, but not in schema`);
}

const fieldResolve = resolverValue[fieldName];
if (typeof fieldResolve !== 'function' && typeof fieldResolve !== 'object') {
throw new Error(`Resolver ${typeName}.${fieldName} must be object or function`);
}
}
});
}
}
});

schema = updateResolversInPlace
? addResolversToExistingSchema({
schema,
resolvers,
defaultFieldResolver,
allowResolversNotInSchema,
})
: createNewSchemaWithResolvers({
schema,
resolvers,
defaultFieldResolver,
allowResolversNotInSchema,
});
? addResolversToExistingSchema(schema, resolvers, defaultFieldResolver)
: createNewSchemaWithResolvers(schema, resolvers, defaultFieldResolver);

checkForResolveTypeResolver(schema, requireResolversForResolveType);

return schema;
}

function addResolversToExistingSchema({
schema,
resolvers,
defaultFieldResolver,
allowResolversNotInSchema,
}: {
schema: GraphQLSchema;
resolvers: IResolvers;
defaultFieldResolver: GraphQLFieldResolver<any, any>;
allowResolversNotInSchema: boolean;
}): GraphQLSchema {
function addResolversToExistingSchema(
schema: GraphQLSchema,
resolvers: IResolvers,
defaultFieldResolver: GraphQLFieldResolver<any, any>
): GraphQLSchema {
const typeMap = schema.getTypeMap();
Object.keys(resolvers).forEach(typeName => {
if (typeName !== '__schema') {
Expand All @@ -143,6 +163,24 @@ function addResolversToExistingSchema({
Object.keys(resolverValue).forEach(fieldName => {
if (fieldName.startsWith('__')) {
type[fieldName.substring(2)] = resolverValue[fieldName];
} else if (fieldName === 'astNode' && type.astNode != null) {
type.astNode = {
...type.astNode,
description: (resolverValue as GraphQLScalarType)?.astNode?.description ?? type.astNode.description,
directives: (type.astNode.directives ?? []).concat(
(resolverValue as GraphQLScalarType)?.astNode?.directives ?? []
),
};
} else if (fieldName === 'extensionASTNodes' && type.extensionASTNodes != null) {
type.extensionASTNodes = ([] ?? type.extensionASTNodes).concat(
(resolverValue as GraphQLScalarType)?.extensionASTNodes ?? []
);
} else if (
fieldName === 'extensions' &&
type.extensions != null &&
(resolverValue as GraphQLScalarType).extensions != null
) {
type.extensions = Object.assign({}, type.extensions, (resolverValue as GraphQLScalarType).extensions);
} else {
type[fieldName] = resolverValue[fieldName];
}
Expand All @@ -154,12 +192,25 @@ function addResolversToExistingSchema({
Object.keys(resolverValue).forEach(fieldName => {
if (fieldName.startsWith('__')) {
config[fieldName.substring(2)] = resolverValue[fieldName];
} else if (!enumValueConfigMap[fieldName]) {
if (allowResolversNotInSchema) {
return;
}
throw new Error(`${type.name}.${fieldName} was defined in resolvers, but not present within ${type.name}`);
} else {
} else if (fieldName === 'astNode' && config.astNode != null) {
config.astNode = {
...config.astNode,
description: (resolverValue as GraphQLScalarType)?.astNode?.description ?? config.astNode.description,
directives: (config.astNode.directives ?? []).concat(
(resolverValue as GraphQLEnumType)?.astNode?.directives ?? []
),
};
} else if (fieldName === 'extensionASTNodes' && config.extensionASTNodes != null) {
config.extensionASTNodes = config.extensionASTNodes.concat(
(resolverValue as GraphQLEnumType)?.extensionASTNodes ?? []
);
} else if (
fieldName === 'extensions' &&
type.extensions != null &&
(resolverValue as GraphQLEnumType).extensions != null
) {
type.extensions = Object.assign({}, type.extensions, (resolverValue as GraphQLEnumType).extensions);
} else if (enumValueConfigMap[fieldName]) {
enumValueConfigMap[fieldName].value = resolverValue[fieldName];
}
});
Expand All @@ -169,15 +220,7 @@ function addResolversToExistingSchema({
Object.keys(resolverValue).forEach(fieldName => {
if (fieldName.startsWith('__')) {
type[fieldName.substring(2)] = resolverValue[fieldName];
return;
}
if (allowResolversNotInSchema) {
return;
}

throw new Error(
`${type.name}.${fieldName} was defined in resolvers, but ${type.name} is not an object or interface type`
);
});
} else if (isObjectType(type) || isInterfaceType(type)) {
Object.keys(resolverValue).forEach(fieldName => {
Expand All @@ -190,23 +233,14 @@ function addResolversToExistingSchema({
const fields = type.getFields();
const field = fields[fieldName];

if (field == null) {
if (allowResolversNotInSchema) {
return;
}

throw new Error(`${typeName}.${fieldName} defined in resolvers, but not in schema`);
}

const fieldResolve = resolverValue[fieldName];
if (typeof fieldResolve === 'function') {
// for convenience. Allows shorter syntax in resolver definition file
field.resolve = fieldResolve;
} else {
if (typeof fieldResolve !== 'object') {
throw new Error(`Resolver ${typeName}.${fieldName} must be object or function`);
if (field != null) {
const fieldResolve = resolverValue[fieldName];
if (typeof fieldResolve === 'function') {
// for convenience. Allows shorter syntax in resolver definition file
field.resolve = fieldResolve;
} else {
setFieldProperties(field, fieldResolve);
}
setFieldProperties(field, fieldResolve);
}
});
}
Expand All @@ -231,17 +265,11 @@ function addResolversToExistingSchema({
return schema;
}

function createNewSchemaWithResolvers({
schema,
resolvers,
defaultFieldResolver,
allowResolversNotInSchema,
}: {
schema: GraphQLSchema;
resolvers: IResolvers;
defaultFieldResolver: GraphQLFieldResolver<any, any>;
allowResolversNotInSchema: boolean;
}): GraphQLSchema {
function createNewSchemaWithResolvers(
schema: GraphQLSchema,
resolvers: IResolvers,
defaultFieldResolver: GraphQLFieldResolver<any, any>
): GraphQLSchema {
schema = mapSchema(schema, {
[MapperKind.SCALAR_TYPE]: type => {
const config = type.toConfig();
Expand All @@ -250,6 +278,24 @@ function createNewSchemaWithResolvers({
Object.keys(resolverValue).forEach(fieldName => {
if (fieldName.startsWith('__')) {
config[fieldName.substring(2)] = resolverValue[fieldName];
} else if (fieldName === 'astNode' && config.astNode != null) {
config.astNode = {
...config.astNode,
description: (resolverValue as GraphQLScalarType)?.astNode?.description ?? config.astNode.description,
directives: (config.astNode.directives ?? []).concat(
(resolverValue as GraphQLScalarType)?.astNode?.directives ?? []
),
};
} else if (fieldName === 'extensionASTNodes' && config.extensionASTNodes != null) {
config.extensionASTNodes = config.extensionASTNodes.concat(
(resolverValue as GraphQLScalarType)?.extensionASTNodes ?? []
);
} else if (
fieldName === 'extensions' &&
config.extensions != null &&
(resolverValue as GraphQLScalarType).extensions != null
) {
config.extensions = Object.assign({}, type.extensions, (resolverValue as GraphQLScalarType).extensions);
} else {
config[fieldName] = resolverValue[fieldName];
}
Expand All @@ -268,12 +314,25 @@ function createNewSchemaWithResolvers({
Object.keys(resolverValue).forEach(fieldName => {
if (fieldName.startsWith('__')) {
config[fieldName.substring(2)] = resolverValue[fieldName];
} else if (!enumValueConfigMap[fieldName]) {
if (allowResolversNotInSchema) {
return;
}
throw new Error(`${type.name}.${fieldName} was defined in resolvers, but not present within ${type.name}`);
} else {
} else if (fieldName === 'astNode' && config.astNode != null) {
config.astNode = {
...config.astNode,
description: (resolverValue as GraphQLScalarType)?.astNode?.description ?? config.astNode.description,
directives: (config.astNode.directives ?? []).concat(
(resolverValue as GraphQLEnumType)?.astNode?.directives ?? []
),
};
} else if (fieldName === 'extensionASTNodes' && config.extensionASTNodes != null) {
config.extensionASTNodes = config.extensionASTNodes.concat(
(resolverValue as GraphQLEnumType)?.extensionASTNodes ?? []
);
} else if (
fieldName === 'extensions' &&
config.extensions != null &&
(resolverValue as GraphQLEnumType).extensions != null
) {
config.extensions = Object.assign({}, type.extensions, (resolverValue as GraphQLEnumType).extensions);
} else if (enumValueConfigMap[fieldName]) {
enumValueConfigMap[fieldName].value = resolverValue[fieldName];
}
});
Expand All @@ -288,17 +347,8 @@ function createNewSchemaWithResolvers({
const config = type.toConfig();
Object.keys(resolverValue).forEach(fieldName => {
if (fieldName.startsWith('__')) {
// this is for isTypeOf and resolveType and all the other stuff.
config[fieldName.substring(2)] = resolverValue[fieldName];
return;
}
if (allowResolversNotInSchema) {
return;
}

throw new Error(
`${type.name}.${fieldName} was defined in resolvers, but ${type.name} is not an object or interface type`
);
});

return new GraphQLUnionType(config);
Expand All @@ -308,22 +358,10 @@ function createNewSchemaWithResolvers({
const resolverValue = resolvers[type.name];
if (resolverValue != null) {
const config = type.toConfig();
const fields = config.fields;

Object.keys(resolverValue).forEach(fieldName => {
if (fieldName.startsWith('__')) {
config[fieldName.substring(2)] = resolverValue[fieldName];
return;
}

const field = fields[fieldName];

if (field == null) {
if (allowResolversNotInSchema) {
return;
}

throw new Error(`${type.name}.${fieldName} defined in resolvers, but not in schema`);
}
});

Expand All @@ -334,22 +372,10 @@ function createNewSchemaWithResolvers({
const resolverValue = resolvers[type.name];
if (resolverValue != null) {
const config = type.toConfig();
const fields = config.fields;

Object.keys(resolverValue).forEach(fieldName => {
if (fieldName.startsWith('__')) {
config[fieldName.substring(2)] = resolverValue[fieldName];
return;
}

const field = fields[fieldName];

if (field == null) {
if (allowResolversNotInSchema) {
return;
}

throw new Error(`${type.name}.${fieldName} defined in resolvers, but not in schema`);
}
});

Expand All @@ -367,9 +393,6 @@ function createNewSchemaWithResolvers({
// for convenience. Allows shorter syntax in resolver definition file
newFieldConfig.resolve = fieldResolve;
} else {
if (typeof fieldResolve !== 'object') {
throw new Error(`Resolver ${typeName}.${fieldName} must be object or function`);
}
setFieldProperties(newFieldConfig, fieldResolve);
}
return newFieldConfig;
Expand Down
29 changes: 29 additions & 0 deletions packages/schema/tests/schemaGenerator.test.ts
View file
Expand Up @@ -838,6 +838,35 @@ describe('generating schema from shorthand', () => {
});
});

test('retains original scalar directives when passing in scalars in resolve functions', () => {
const schema = makeExecutableSchema({
typeDefs: `
directive @test on SCALAR
scalar Test @test
type Query {
test: Test
}
`,
resolvers: {
Test: new GraphQLScalarType({
name: 'Test',
description: 'Test resolver',
serialize: (value) => value,
parseValue: (value) => value,
}),
Query: {
test: () => 42,
},
},
});

const testType = schema.getType('Test');
expect(testType).toBeInstanceOf(GraphQLScalarType);
expect(testType.astNode.directives.length).toBe(1);
});

test('retains scalars after walking/recreating the schema', () => {
const shorthand = `
scalar Test
Expand Down

0 comments on commit 5b93e0e

Please sign in to comment.