-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Decouple typescript-operations plugin from typescript plugin
#10572
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
Changes from all commits
aa3f750
69f6ccd
c999e12
fccc3b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@graphql-codegen/typescript-operations": patch | ||
| --- | ||
| dependencies updates: | ||
| - Removed dependency [`@graphql-codegen/typescript@^5.0.7` ↗︎](https://www.npmjs.com/package/@graphql-codegen/typescript/v/5.0.7) (from `dependencies`) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| '@graphql-codegen/typescript-operations': major | ||
| --- | ||
|
|
||
| BREAKING CHANGE: Decouple `typescript-operations` plugin from `typescript` plugin | ||
|
|
||
| Previously, `TypeScriptOperationVariablesToObject` from `typescript-operations` was extending from `typescript` plugin. This made it (1) very hard to read, as we need to jump from base class <-> typescript class <-> typescript-operations class to understand the flow and (2) very hard to evolve the two independently (which is the point of this work). | ||
|
|
||
| Since there's not much shared logic anyways, it's simpler to extend the `typescript-operations` class from the base class directly. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,11 @@ | ||
| import { TypeScriptOperationVariablesToObject as TSOperationVariablesToObject } from '@graphql-codegen/typescript'; | ||
| import { | ||
| OperationVariablesToObject, | ||
| ConvertNameFn, | ||
| NormalizedAvoidOptionalsConfig, | ||
| NormalizedScalarsMap, | ||
| ParsedEnumValuesMap, | ||
| } from '@graphql-codegen/visitor-plugin-common'; | ||
| import { Kind, TypeNode } from 'graphql'; | ||
|
|
||
| export const SCALARS = { | ||
| ID: 'string | number', | ||
|
|
@@ -10,7 +17,36 @@ export const SCALARS = { | |
|
|
||
| const MAYBE_SUFFIX = ' | null'; | ||
|
|
||
| export class TypeScriptOperationVariablesToObject extends TSOperationVariablesToObject { | ||
| export class TypeScriptOperationVariablesToObject extends OperationVariablesToObject { | ||
| constructor( | ||
| _scalars: NormalizedScalarsMap, | ||
| _convertName: ConvertNameFn, | ||
| private _avoidOptionals: NormalizedAvoidOptionalsConfig, | ||
| private _immutableTypes: boolean, | ||
| _namespacedImportName: string | null, | ||
| _enumNames: string[], | ||
| _enumPrefix: boolean, | ||
| _enumSuffix: boolean, | ||
| _enumValues: ParsedEnumValuesMap, | ||
| _applyCoercion: boolean | ||
|
Comment on lines
+22
to
+31
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are lots of default values here from the Since we are really dedicating this implementation for We event removed the last param |
||
| ) { | ||
| super( | ||
| _scalars, | ||
| _convertName, | ||
| _namespacedImportName, | ||
| _enumNames, | ||
| _enumPrefix, | ||
| _enumSuffix, | ||
| _enumValues, | ||
| _applyCoercion, | ||
| {} | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is We were receiving |
||
| ); | ||
| } | ||
|
|
||
| protected formatFieldString(fieldName: string, isNonNullType: boolean, hasDefaultValue: boolean): string { | ||
| return `${fieldName}${this.getAvoidOption(isNonNullType, hasDefaultValue) ? '?' : ''}`; | ||
| } | ||
|
|
||
| protected formatTypeString(fieldType: string, _isNonNullType: boolean, _hasDefaultValue: boolean): string { | ||
| return fieldType; | ||
| } | ||
|
|
@@ -23,11 +59,37 @@ export class TypeScriptOperationVariablesToObject extends TSOperationVariablesTo | |
| return str; | ||
| } | ||
|
|
||
| protected getAvoidOption(isNonNullType: boolean, hasDefaultValue: boolean) { | ||
| const options = this._avoidOptionals; | ||
| return ((options.object || !options.defaultValue) && hasDefaultValue) || (!options.object && !isNonNullType); | ||
| } | ||
|
|
||
| public wrapAstTypeWithModifiers(baseType: string, typeNode: TypeNode, applyCoercion = false): string { | ||
| if (typeNode.kind === Kind.NON_NULL_TYPE) { | ||
| const type = this.wrapAstTypeWithModifiers(baseType, typeNode.type, applyCoercion); | ||
|
|
||
| return this.clearOptional(type); | ||
| } | ||
| if (typeNode.kind === Kind.LIST_TYPE) { | ||
| const innerType = this.wrapAstTypeWithModifiers(baseType, typeNode.type, applyCoercion); | ||
| const listInputCoercionExtension = applyCoercion ? ` | ${innerType}` : ''; | ||
|
|
||
| return this.wrapMaybe( | ||
| `${this._immutableTypes ? 'ReadonlyArray' : 'Array'}<${innerType}>${listInputCoercionExtension}` | ||
| ); | ||
| } | ||
| return this.wrapMaybe(baseType); | ||
| } | ||
|
|
||
| protected wrapMaybe(type: string): string { | ||
| return type?.endsWith(MAYBE_SUFFIX) ? type : `${type}${MAYBE_SUFFIX}`; | ||
| } | ||
|
|
||
| protected getScalar(name: string): string { | ||
| return this._scalars?.[name]?.input ?? SCALARS[name] ?? 'unknown'; | ||
| } | ||
|
|
||
| protected getPunctuation(): string { | ||
| return ';'; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -169,9 +169,7 @@ export class TypeScriptDocumentsVisitor extends BaseDocumentsVisitor< | |
| this.config.enumPrefix, | ||
| this.config.enumSuffix, | ||
| this.config.enumValues, | ||
| this.config.arrayInputCoercion, | ||
| undefined, | ||
| undefined | ||
|
Comment on lines
-173
to
-174
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we are always passing undefined through, we can just use the default. |
||
| this.config.arrayInputCoercion | ||
| ) | ||
| ); | ||
| this._declarationBlockConfig = { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bye! 🫡
@graphql-codegen/typescriptserved us well for the last 10+ years. But with goal of creating the independence oftypescriptfortypescript-operationsplugin for this work, we can now remove this dep.