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.

Sign up for GitHub

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

Jump to bottom

addResolversToSchema should not lose directives #1623

Merged
merged 4 commits into from Jun 10, 2020
Merged

addResolversToSchema should not lose directives #1623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
55c3763
Bump @typescript-eslint/eslint-plugin from 3.1.0 to 3.2.0 (#1620)
dependabot[bot] Jun 9, 2020
34c1f96
consolidate resolvers validation
yaacovCR Jun 9, 2020
c91cad9
addResolversToSchema should merge scalar/enum resolvers
yaacovCR Jun 9, 2020
69d5004
perform shallow merge only for extensions
yaacovCR Jun 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Expand Up @@ -43,7 +43,7 @@
"jest": "26.0.1",
"ts-jest": "26.1.0",
"typescript": "3.9.5",
"@typescript-eslint/eslint-plugin": "3.1.0",
"@typescript-eslint/eslint-plugin": "3.2.0",
"@typescript-eslint/parser": "3.2.0",
"bob-the-bundler": "1.0.2",
"eslint": "7.2.0",
Expand Down
235 changes: 129 additions & 106 deletions packages/schema/src/addResolversToSchema.ts
View file
Open in desktop
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
Open in desktop
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