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

Add root level resolvers support to avoidOptionals #10077

Merged
merged 4 commits into from
Aug 3, 2024
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
11 changes: 11 additions & 0 deletions .changeset/short-mirrors-fail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
'@graphql-codegen/visitor-plugin-common': minor
'@graphql-codegen/typescript-operations': minor
'@graphql-codegen/typescript': minor
'@graphql-codegen/typescript-resolvers': minor
---

Extend `config.avoidOptions` to support query, mutation and subscription

Previously, `config.avoidOptions.resolvers` was being used to make query, mutation and subscription fields non-optional.
Now, `config.avoidOptions.query`, `config.avoidOptions.mutation` and `config.avoidOptions.subscription` can be used to target the respective types.
Original file line number Diff line number Diff line change
@@ -1,21 +1,29 @@
import { AvoidOptionalsConfig } from './types.js';
import { AvoidOptionalsConfig, NormalizedAvoidOptionalsConfig } from './types.js';

export const DEFAULT_AVOID_OPTIONALS: AvoidOptionalsConfig = {
export const DEFAULT_AVOID_OPTIONALS: NormalizedAvoidOptionalsConfig = {
object: false,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NormalizedAvoidOptionalsConfig is similar to AvoidOptionalsConfig but all the fields are non-optional, making it much easier to code with

inputValue: false,
field: false,
defaultValue: false,
resolvers: false,
query: false,
mutation: false,
subscription: false,
};

export function normalizeAvoidOptionals(avoidOptionals?: boolean | AvoidOptionalsConfig): AvoidOptionalsConfig {
export function normalizeAvoidOptionals(
avoidOptionals?: boolean | AvoidOptionalsConfig
): NormalizedAvoidOptionalsConfig {
if (typeof avoidOptionals === 'boolean') {
return {
object: avoidOptionals,
inputValue: avoidOptionals,
field: avoidOptionals,
defaultValue: avoidOptionals,
resolvers: avoidOptionals,
query: avoidOptionals,
mutation: avoidOptionals,
subscription: avoidOptionals,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
ConvertOptions,
DeclarationKind,
EnumValuesMap,
NormalizedAvoidOptionalsConfig,
NormalizedScalarsMap,
ParsedEnumValuesMap,
ResolversNonOptionalTypenameConfig,
Expand All @@ -50,6 +51,7 @@ import {
wrapTypeWithModifiers,
} from './utils.js';
import { OperationVariablesToObject } from './variables-to-object.js';
import { normalizeAvoidOptionals } from './avoid-optionals.js';

export interface ParsedResolversConfig extends ParsedConfig {
contextType: ParsedMapper;
Expand All @@ -60,7 +62,7 @@ export interface ParsedResolversConfig extends ParsedConfig {
[typeName: string]: ParsedMapper;
};
defaultMapper: ParsedMapper | null;
avoidOptionals: AvoidOptionalsConfig | boolean;
avoidOptionals: NormalizedAvoidOptionalsConfig;
addUnderscoreToArgsType: boolean;
enumValues: ParsedEnumValuesMap;
resolverTypeWrapperSignature: string;
Expand All @@ -78,6 +80,8 @@ export interface ParsedResolversConfig extends ParsedConfig {
resolversNonOptionalTypename: ResolversNonOptionalTypenameConfig;
}

type FieldDefinitionPrintFn = (parentName: string, avoidResolverOptionals: boolean) => string | null;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was REALLY confused why/how the field's name is a function. Found out it's because we do custom logic in FieldDefinition function. I'm extracting the type here to be able to type it instead of using any


export interface RawResolversConfig extends RawConfig {
/**
* @description Adds `_` to generated `Args` types in order to avoid duplicate identifiers.
Expand Down Expand Up @@ -429,6 +433,9 @@ export interface RawResolversConfig extends RawConfig {
* inputValue: true,
* object: true,
* defaultValue: true,
* query: true,
* mutation: true,
* subscription: true,
* }
* },
* },
Expand Down Expand Up @@ -682,7 +689,7 @@ export class BaseResolversVisitor<
allResolversTypeName: getConfigValue(rawConfig.allResolversTypeName, 'Resolvers'),
rootValueType: parseMapper(rawConfig.rootValueType || '{}', 'RootValueType'),
namespacedImportName: getConfigValue(rawConfig.namespacedImportName, ''),
avoidOptionals: getConfigValue(rawConfig.avoidOptionals, false),
avoidOptionals: normalizeAvoidOptionals(rawConfig.avoidOptionals),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This plugin seems to be the only one that's not using normalizeAvoidOptionals. This makes it consistent.

defaultMapper: rawConfig.defaultMapper
? parseMapper(rawConfig.defaultMapper || 'any', 'DefaultMapperType')
: null,
Expand Down Expand Up @@ -1307,7 +1314,7 @@ export class BaseResolversVisitor<
}

protected formatRootResolver(schemaTypeName: string, resolverType: string, declarationKind: DeclarationKind): string {
return `${schemaTypeName}${this.config.avoidOptionals ? '' : '?'}: ${resolverType}${this.getPunctuation(
return `${schemaTypeName}${this.config.avoidOptionals.resolvers ? '' : '?'}: ${resolverType}${this.getPunctuation(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old implementation can cause a scenario where an empty object is passed in, but it'd be treated as truthy so it'd treat it as non-optional.

In this new implementation, we use resolvers because it's the option that is used for all resolvers-related fields in the past.

declarationKind
)}`;
}
Expand Down Expand Up @@ -1394,11 +1401,11 @@ export class BaseResolversVisitor<
return `ParentType extends ${parentType} = ${parentType}`;
}

FieldDefinition(node: FieldDefinitionNode, key: string | number, parent: any): (parentName: string) => string | null {
FieldDefinition(node: FieldDefinitionNode, key: string | number, parent: any): FieldDefinitionPrintFn {
const hasArguments = node.arguments && node.arguments.length > 0;
const declarationKind = 'type';

return (parentName: string) => {
return (parentName, avoidResolverOptionals) => {
const original: FieldDefinitionNode = parent[key];
const baseType = getBaseTypeNode(original.type);
const realType = baseType.name.value;
Expand Down Expand Up @@ -1431,10 +1438,7 @@ export class BaseResolversVisitor<
)
: null;

const avoidInputsOptionals =
typeof this.config.avoidOptionals === 'object'
? this.config.avoidOptionals?.inputValue
: this.config.avoidOptionals === true;
const avoidInputsOptionals = this.config.avoidOptionals.inputValue;

if (argsType !== null) {
const argsToForceRequire = original.arguments.filter(
Expand Down Expand Up @@ -1463,10 +1467,6 @@ export class BaseResolversVisitor<

const resolverType = isSubscriptionType ? 'SubscriptionResolver' : directiveMappings[0] ?? 'Resolver';

const avoidResolverOptionals =
typeof this.config.avoidOptionals === 'object'
? this.config.avoidOptionals?.resolvers
: this.config.avoidOptionals === true;
Comment on lines -1466 to -1469
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now pass this from caller. I found that this function is called in Interface fields and Object type fields

const signature: {
name: string;
modifier: string;
Expand Down Expand Up @@ -1532,15 +1532,31 @@ export class BaseResolversVisitor<
});
const typeName = node.name as any as string;
const parentType = this.getParentTypeToUse(typeName);
const isRootType = [
this.schema.getQueryType()?.name,
this.schema.getMutationType()?.name,
this.schema.getSubscriptionType()?.name,
].includes(typeName);

const fieldsContent = node.fields.map((f: any) => f(node.name));
const rootType = ((): false | 'query' | 'mutation' | 'subscription' => {
if (this.schema.getQueryType()?.name === typeName) {
return 'query';
}
if (this.schema.getMutationType()?.name === typeName) {
return 'mutation';
}
if (this.schema.getSubscriptionType()?.name === typeName) {
return 'subscription';
}
return false;
})();

const fieldsContent = (node.fields as unknown as FieldDefinitionPrintFn[]).map(f => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to inline node.fields as unknown as FieldDefinitionPrintFn[] here because it's the easiest way to communicate where this function comes from. If there's a better way, let me know!

return f(
typeName,
(rootType === 'query' && this.config.avoidOptionals.query) ||
(rootType === 'mutation' && this.config.avoidOptionals.mutation) ||
(rootType === 'subscription' && this.config.avoidOptionals.subscription) ||
(rootType === false && this.config.avoidOptionals.resolvers)
);
});

if (!isRootType) {
if (!rootType) {
fieldsContent.push(
indent(
`${
Expand Down Expand Up @@ -1720,21 +1736,23 @@ export class BaseResolversVisitor<
const allTypesMap = this._schema.getTypeMap();
const implementingTypes: string[] = [];

this._collectedResolvers[node.name as any] = {
const typeName = node.name as any as string;
Copy link
Collaborator Author

@eddeee888 eddeee888 Aug 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleaning this up by limiting where we do as any as string in this function


this._collectedResolvers[typeName] = {
typename: name + '<ContextType>',
baseGeneratedTypename: name,
};

for (const graphqlType of Object.values(allTypesMap)) {
if (graphqlType instanceof GraphQLObjectType) {
const allInterfaces = graphqlType.getInterfaces();
if (allInterfaces.find(int => int.name === (node.name as any as string))) {
if (allInterfaces.find(int => int.name === typeName)) {
implementingTypes.push(graphqlType.name);
}
}
}

const parentType = this.getParentTypeToUse(node.name as any as string);
const parentType = this.getParentTypeToUse(typeName);
const possibleTypes = implementingTypes.map(name => `'${name}'`).join(' | ') || 'null';
const fields = this.config.onlyResolveTypeForInterfaces ? [] : node.fields || [];

Expand All @@ -1749,7 +1767,9 @@ export class BaseResolversVisitor<
this.config.optionalResolveType ? '?' : ''
}: TypeResolveFn<${possibleTypes}, ParentType, ContextType>${this.getPunctuation(declarationKind)}`
),
...fields.map((f: any) => f(node.name)),
...(fields as unknown as FieldDefinitionPrintFn[]).map(f =>
f(typeName, this.config.avoidOptionals.resolvers)
),
].join('\n')
).string;
}
Expand Down Expand Up @@ -1809,7 +1829,7 @@ export class BaseResolversVisitor<
return null;
}

const addOptionalSign = !this.config.avoidOptionals && !isNonNullType(field.type);
const addOptionalSign = !this.config.avoidOptionals.resolvers && !isNonNullType(field.type);

return {
addOptionalSign,
Expand Down
5 changes: 5 additions & 0 deletions packages/plugins/other/visitor-plugin-common/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,13 @@ export interface AvoidOptionalsConfig {
inputValue?: boolean;
defaultValue?: boolean;
resolvers?: boolean;
query?: boolean;
mutation?: boolean;
subscription?: boolean;
}

export type NormalizedAvoidOptionalsConfig = Required<AvoidOptionalsConfig>;

export interface ParsedImport {
moduleName: string | null;
propName: string;
Expand Down
6 changes: 3 additions & 3 deletions packages/plugins/typescript/operations/src/visitor.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import {
AvoidOptionalsConfig,
BaseDocumentsVisitor,
DeclarationKind,
generateFragmentImportStatement,
getConfigValue,
LoadedFragment,
normalizeAvoidOptionals,
NormalizedAvoidOptionalsConfig,
ParsedDocumentsConfig,
PreResolveTypesProcessor,
SelectionSetProcessorConfig,
Expand All @@ -20,7 +20,7 @@ import { TypeScriptSelectionSetProcessor } from './ts-selection-set-processor.js

export interface TypeScriptDocumentsParsedConfig extends ParsedDocumentsConfig {
arrayInputCoercion: boolean;
avoidOptionals: AvoidOptionalsConfig;
avoidOptionals: NormalizedAvoidOptionalsConfig;
immutableTypes: boolean;
noExport: boolean;
maybeValue: string;
Expand Down Expand Up @@ -108,7 +108,7 @@ export class TypeScriptDocumentsVisitor extends BaseDocumentsVisitor<
new TypeScriptOperationVariablesToObject(
this.scalars,
this.convertName.bind(this),
this.config.avoidOptionals.object,
this.config.avoidOptionals,
this.config.immutableTypes,
this.config.namespacedImportName,
enumsNames,
Expand Down
10 changes: 4 additions & 6 deletions packages/plugins/typescript/resolvers/src/visitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
BaseResolversVisitor,
DeclarationKind,
getConfigValue,
normalizeAvoidOptionals,
ParsedResolversConfig,
} from '@graphql-codegen/visitor-plugin-common';
import autoBind from 'auto-bind';
Expand Down Expand Up @@ -34,7 +35,7 @@ export class TypeScriptResolversVisitor extends BaseResolversVisitor<
super(
pluginConfig,
{
avoidOptionals: getConfigValue(pluginConfig.avoidOptionals, false),
avoidOptionals: normalizeAvoidOptionals(pluginConfig.avoidOptionals),
useIndexSignature: getConfigValue(pluginConfig.useIndexSignature, false),
wrapFieldDefinitions: getConfigValue(pluginConfig.wrapFieldDefinitions, false),
allowParentTypeOverride: getConfigValue(pluginConfig.allowParentTypeOverride, false),
Expand Down Expand Up @@ -75,10 +76,7 @@ export class TypeScriptResolversVisitor extends BaseResolversVisitor<
}

protected formatRootResolver(schemaTypeName: string, resolverType: string, declarationKind: DeclarationKind): string {
const avoidOptionals =
typeof this.config.avoidOptionals === 'object'
? this.config.avoidOptionals?.resolvers
: !!this.config.avoidOptionals === true;
const avoidOptionals = this.config.avoidOptionals.resolvers;
return `${schemaTypeName}${avoidOptionals ? '' : '?'}: ${resolverType}${this.getPunctuation(declarationKind)}`;
}

Expand Down Expand Up @@ -121,7 +119,7 @@ export class TypeScriptResolversVisitor extends BaseResolversVisitor<

protected buildEnumResolverContentBlock(node: EnumTypeDefinitionNode, mappedEnumType: string): string {
const valuesMap = `{ ${(node.values || [])
.map(v => `${v.name as any as string}${this.config.avoidOptionals ? '' : '?'}: any`)
.map(v => `${v.name as any as string}${this.config.avoidOptionals.resolvers ? '' : '?'}: any`)
.join(', ')} }`;

this._globalDeclarations.add(ENUM_RESOLVERS_SIGNATURE);
Expand Down
Loading
Loading