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

[typescript-resolvers] Extract union types to ResolversUnionTypes #9069

Merged
merged 15 commits into from
Mar 6, 2023
Merged

[typescript-resolvers] Extract union types to ResolversUnionTypes #9069

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
dc52ffe
Build ResolversUnionTypes without referencing it
eddeee888 Feb 24, 2023
cc5b934
Update ResolversTypes and ResolversParentTypes to link to ResolversUn…
eddeee888 Feb 25, 2023
77592f5
Ensure schema type names are referenced correctly in ResolversUnionTy…
eddeee888 Feb 25, 2023
02d2624
Do not generate ResolversUnionTypes if there is no union
eddeee888 Feb 25, 2023
92445a0
Do not test comment generated for ResolversUnionTypes
eddeee888 Feb 25, 2023
3951756
Handle mappers and defaultMapper with placeholders for union
eddeee888 Feb 25, 2023
669ef3b
Fix Omit in ResolversUnionTypes and tests
eddeee888 Feb 26, 2023
2caf69b
Clean up
eddeee888 Feb 26, 2023
43cffcf
Abstract getRelevantFieldsToOmit
eddeee888 Feb 27, 2023
869aef4
Ensure backward compatibility when using defaultMappers with placeholder
eddeee888 Feb 27, 2023
09b4d59
Ensure union types go through federation and handle applyNamespacedIm…
eddeee888 Feb 28, 2023
6dd816c
Fix dev-test
eddeee888 Feb 28, 2023
a2d0369
Add changeset
eddeee888 Mar 6, 2023
21132ad
Merge branch 'master' into typescript-resolvers-refactor-union-types
eddeee888 Mar 6, 2023
7758126
Remove comment
eddeee888 Mar 6, 2023
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
9 changes: 7 additions & 2 deletions dev-test/modules/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ export type DirectiveResolverFn<TResult = {}, TParent = {}, TContext = {}, TArgs
info: GraphQLResolveInfo
) => TResult | Promise<TResult>;

/** Mapping of union types */
export type ResolversUnionTypes = {
PaymentOption: CreditCard | Paypal;
};

/** Mapping between all available schema types and the resolvers types */
export type ResolversTypes = {
Article: ResolverTypeWrapper<Article>;
Expand All @@ -173,7 +178,7 @@ export type ResolversTypes = {
ID: ResolverTypeWrapper<Scalars['ID']>;
Int: ResolverTypeWrapper<Scalars['Int']>;
Mutation: ResolverTypeWrapper<{}>;
PaymentOption: ResolversTypes['CreditCard'] | ResolversTypes['Paypal'];
PaymentOption: ResolverTypeWrapper<ResolversUnionTypes['PaymentOption']>;
Paypal: ResolverTypeWrapper<Paypal>;
Query: ResolverTypeWrapper<{}>;
String: ResolverTypeWrapper<Scalars['String']>;
Expand All @@ -193,7 +198,7 @@ export type ResolversParentTypes = {
ID: Scalars['ID'];
Int: Scalars['Int'];
Mutation: {};
PaymentOption: ResolversParentTypes['CreditCard'] | ResolversParentTypes['Paypal'];
PaymentOption: ResolversUnionTypes['PaymentOption'];
Paypal: Paypal;
Query: {};
String: Scalars['String'];
Expand Down
192 changes: 143 additions & 49 deletions packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,8 @@ export class BaseResolversVisitor<
protected _usedMappers: { [key: string]: boolean } = {};
protected _resolversTypes: ResolverTypes = {};
protected _resolversParentTypes: ResolverParentTypes = {};
protected _hasReferencedResolversUnionTypes = false;
protected _resolversUnionTypes: Record<string, string> = {};
protected _rootTypeNames = new Set<string>();
protected _globalDeclarations = new Set<string>();
protected _federation: ApolloFederation;
Expand Down Expand Up @@ -624,6 +626,7 @@ export class BaseResolversVisitor<
name => this.getParentTypeToUse(name),
namedType => !isEnumType(namedType)
);
this._resolversUnionTypes = this.createResolversUnionTypes();
this._fieldContextTypeMap = this.createFieldContextTypeMap();
this._directiveContextTypesMap = this.createDirectivedContextType();
this._directiveResolverMappings = rawConfig.directiveResolverMappings ?? {};
Expand Down Expand Up @@ -754,10 +757,9 @@ export class BaseResolversVisitor<
} else if (isScalar) {
prev[typeName] = applyWrapper(this._getScalar(typeName));
} else if (isUnionType(schemaType)) {
prev[typeName] = schemaType
.getTypes()
.map(type => getTypeToUse(type.name))
.join(' | ');
this._hasReferencedResolversUnionTypes = true;
const resolversType = this.convertName('ResolversUnionTypes');
prev[typeName] = applyWrapper(`${resolversType}['${typeName}']`);
} else if (isEnumType(schemaType)) {
prev[typeName] = this.convertName(typeName, { useTypesPrefix: this.config.enumPrefix }, true);
} else {
Expand All @@ -766,44 +768,11 @@ export class BaseResolversVisitor<
}

if (shouldApplyOmit && prev[typeName] !== 'any' && isObjectType(schemaType)) {
const fields = schemaType.getFields();
const relevantFields: {
addOptionalSign: boolean;
fieldName: string;
replaceWithType: string;
}[] = this._federation
.filterFieldNames(Object.keys(fields))
.filter(fieldName => {
const field = fields[fieldName];
const baseType = getBaseType(field.type);

// Filter out fields of types that are not included
if (shouldInclude && !shouldInclude(baseType)) {
return false;
}
return true;
})
.map(fieldName => {
const field = fields[fieldName];
const baseType = getBaseType(field.type);
const isUnion = isUnionType(baseType);

if (!this.config.mappers[baseType.name] && !isUnion && !this._shouldMapType[baseType.name]) {
return null;
}

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

return {
addOptionalSign,
fieldName,
replaceWithType: wrapTypeWithModifiers(getTypeToUse(baseType.name), field.type, {
wrapOptional: this.applyMaybe,
wrapArray: this.wrapWithArray,
}),
};
})
.filter(a => a);
const relevantFields = this.getRelevantFieldsToOmit({
schemaType,
getTypeToUse,
shouldInclude,
});

if (relevantFields.length > 0) {
// Puts ResolverTypeWrapper on top of an entire type
Expand All @@ -819,15 +788,15 @@ export class BaseResolversVisitor<
}

if (!isMapped && hasDefaultMapper && hasPlaceholder(this.config.defaultMapper.type)) {
// Make sure the inner type has no ResolverTypeWrapper
const name = clearWrapper(isScalar ? this._getScalar(typeName) : prev[typeName]);
const replaced = replacePlaceholder(this.config.defaultMapper.type, name);
const originalTypeName = isScalar ? this._getScalar(typeName) : prev[typeName];

// Don't wrap Union with ResolverTypeWrapper, each inner type already has it
if (isUnionType(schemaType)) {
prev[typeName] = replaced;
// Don't clear ResolverTypeWrapper from Unions
prev[typeName] = replacePlaceholder(this.config.defaultMapper.type, originalTypeName);
} else {
prev[typeName] = applyWrapper(replacePlaceholder(this.config.defaultMapper.type, name));
const name = clearWrapper(originalTypeName);
const replaced = replacePlaceholder(this.config.defaultMapper.type, name);
prev[typeName] = applyWrapper(replaced);
Comment on lines +791 to +799
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Previous logic suggests that it's possible for a union to be in the scalar map 🤔 I'm unsure when/if it's possible.

However, the updated logic should be backward compatible as it generates the same partial and wrapper order.

}
}

Expand All @@ -837,7 +806,7 @@ export class BaseResolversVisitor<

protected replaceFieldsInType(
typeName: string,
relevantFields: { addOptionalSign: boolean; fieldName: string; replaceWithType: string }[]
relevantFields: ReturnType<typeof this.getRelevantFieldsToOmit>
): string {
this._globalDeclarations.add(OMIT_TYPE);
return `Omit<${typeName}, ${relevantFields.map(f => `'${f.fieldName}'`).join(' | ')}> & { ${relevantFields
Expand Down Expand Up @@ -880,6 +849,64 @@ export class BaseResolversVisitor<
return `Array<${t}>`;
}

protected createResolversUnionTypes(): Record<string, string> {
Copy link
Collaborator Author

@eddeee888 eddeee888 Feb 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

createResolversUnionTypes is the simplified version of createResolversFields that is targeted for Unions.
I chose to create a new function because createResolversFields is quite large and rigid so it's hard to implement/maintain

if (!this._hasReferencedResolversUnionTypes) {
return {};
}

const allSchemaTypes = this._schema.getTypeMap();
const typeNames = this._federation.filterTypeNames(Object.keys(allSchemaTypes));

const unionTypes = typeNames.reduce((res, typeName) => {
const schemaType = allSchemaTypes[typeName];

if (isUnionType(schemaType)) {
const referencedTypes = schemaType.getTypes().map(unionMemberType => {
const isUnionMemberMapped = this.config.mappers[unionMemberType.name];

// 1. If mapped without placehoder, just use it without doing extra checks
if (isUnionMemberMapped && !hasPlaceholder(isUnionMemberMapped.type)) {
return isUnionMemberMapped.type;
}

// 2. Work out value for union member type
// 2a. By default, use the typescript type
let unionMemberValue = this.convertName(unionMemberType.name, {}, true);

// 2b. Find fields to Omit if needed.
// - If no field to Omit, "type with maybe Omit" is typescript type i.e. no Omit
// - If there are fields to Omit, "type with maybe Omit"
const fieldsToOmit = this.getRelevantFieldsToOmit({
schemaType: unionMemberType,
getTypeToUse: this.getTypeToUse,
});
if (fieldsToOmit.length > 0) {
unionMemberValue = this.replaceFieldsInType(unionMemberValue, fieldsToOmit);
}

// 2c. If union member is mapped with placeholder, use the "type with maybe Omit" as {T}
if (isUnionMemberMapped && hasPlaceholder(isUnionMemberMapped.type)) {
return replacePlaceholder(isUnionMemberMapped.type, unionMemberValue);
}

// 2d. If has default mapper with placeholder, use the "type with maybe Omit" as {T}
const hasDefaultMapper = !!this.config.defaultMapper?.type;
const isScalar = this.config.scalars[typeName];
if (hasDefaultMapper && hasPlaceholder(this.config.defaultMapper.type)) {
const finalTypename = isScalar ? this._getScalar(typeName) : unionMemberValue;
return replacePlaceholder(this.config.defaultMapper.type, finalTypename);
}

return unionMemberValue;
});
res[typeName] = referencedTypes.map(type => `( ${type} )`).join(' | '); // Must wrap every union member in explicit "( )" to separate the members
Copy link
Collaborator Author

@eddeee888 eddeee888 Feb 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Must wrap every union member in explicit "( )" to separate the members

Result usually looks like this:

export type ResolversUnionTypes =  {
  UnionThing: ( TypeA ) | ( TypeB )
}

Prettier should remove the round brackets automatically i.e. result will be UnionThing: TypeA | TypeB

I've added it this way so it's a bit easier to add types to the union members without ambiguity e.g.

// This is ambiguous
UnionThing: TypeA & { __typename: 'TypeA' } | TypeB & { __typename: 'TypeB' }

// Prettier will try to turn it into this:
UnionThing: ( TypeA & { __typename: 'TypeA' } ) | ( TypeB & { __typename: 'TypeB' } )

}
return res;
}, {});

return unionTypes;
}

protected createFieldContextTypeMap(): FieldContextTypeMap {
return this.config.fieldContextTypes.reduce<FieldContextTypeMap>((prev, fieldContextType) => {
const items = fieldContextType.split('#');
Expand Down Expand Up @@ -935,6 +962,24 @@ export class BaseResolversVisitor<
).string;
}

public buildResolversUnionTypes(): string {
if (Object.keys(this._resolversUnionTypes).length === 0) {
return '';
}

const declarationKind = 'type';
return new DeclarationBlock(this._declarationBlockConfig)
.export()
.asKind(declarationKind)
.withName(this.convertName('ResolversUnionTypes'))
.withComment('Mapping of union types')
.withBlock(
Object.entries(this._resolversUnionTypes)
.map(([typeName, value]) => indent(`${typeName}: ${value}${this.getPunctuation(declarationKind)}`))
.join('\n')
).string;
}

public get schema(): GraphQLSchema {
return this._schema;
}
Expand Down Expand Up @@ -1478,6 +1523,55 @@ export class BaseResolversVisitor<
SchemaDefinition() {
return null;
}

private getRelevantFieldsToOmit({
schemaType,
shouldInclude,
getTypeToUse,
}: {
schemaType: GraphQLObjectType;
getTypeToUse: (name: string) => string;
shouldInclude?: (type: GraphQLNamedType) => boolean;
}): {
addOptionalSign: boolean;
fieldName: string;
replaceWithType: string;
}[] {
const fields = schemaType.getFields();
return this._federation
.filterFieldNames(Object.keys(fields))
.filter(fieldName => {
const field = fields[fieldName];
const baseType = getBaseType(field.type);

// Filter out fields of types that are not included
if (shouldInclude && !shouldInclude(baseType)) {
return false;
}
return true;
})
.map(fieldName => {
const field = fields[fieldName];
const baseType = getBaseType(field.type);
const isUnion = isUnionType(baseType);

if (!this.config.mappers[baseType.name] && !isUnion && !this._shouldMapType[baseType.name]) {
return null;
}

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

return {
addOptionalSign,
fieldName,
replaceWithType: wrapTypeWithModifiers(getTypeToUse(baseType.name), field.type, {
wrapOptional: this.applyMaybe,
wrapArray: this.wrapWithArray,
}),
};
})
.filter(a => a);
}
}

function replacePlaceholder(pattern: string, typename: string): string {
Expand Down
2 changes: 2 additions & 0 deletions packages/plugins/typescript/resolvers/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ export type DirectiveResolverFn<TResult = {}, TParent = {}, TContext = {}, TArgs

const resolversTypeMapping = visitor.buildResolversTypes();
const resolversParentTypeMapping = visitor.buildResolversParentTypes();
const resolversUnionTypesMapping = visitor.buildResolversUnionTypes();
const { getRootResolver, getAllDirectiveResolvers, mappersImports, unusedMappers, hasScalars } = visitor;

if (hasScalars()) {
Expand Down Expand Up @@ -282,6 +283,7 @@ export type DirectiveResolverFn<TResult = {}, TParent = {}, TContext = {}, TArgs
prepend,
content: [
header,
resolversUnionTypesMapping,
resolversTypeMapping,
resolversParentTypeMapping,
...visitorResult.definitions.filter(d => typeof d === 'string'),
Expand Down
Loading