-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Adjust config inheritance to reflect usage and type-hint better #10560
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
Conversation
- addTypename/skipTypename: is only a types-visitor concern. This is moved to types-visitor from base-visitor - nonOptionalTypename: is a documents-visitor and types-visitor concern. Moved from base-visitor there - extractAllFieldsToTypes: is a documents-visitor concern. Moved from base-visitor there - enumPrefix and enumSuffix: need to be in base-visitor as all 3 types of visitors need this to correctly sync the enum type names. This is moved to base visitor - ignoreEnumValuesFromSchema: is a documents-visitor and types-visitor concern. Moved from base-visitor there. - globalNamespace: is a documents-visitor concern. Moved from base-visitor there Other changes: - documents-visitor no longer extends types-visitor _option types_ as they have two distinct usages now. The types now extend base-visitor types. This is now consistent with documents-visitor extending base-visitor - Classes now handle config parsing and types at the same level e.g. if typescript-operations plugin parses configOne, then the types for configOne must be in that class, rather than in base-documents-visitor
🦋 Changeset detectedLatest commit: f6dd8aa The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🚀 Snapshot Release (
|
| Package | Version | Info |
|---|---|---|
@graphql-codegen/cli |
6.0.3-alpha-20260105125118-f6dd8aad76b522a60a52fee9cc56d3e46fc3c9a5 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/introspection |
5.0.1-alpha-20260105125118-f6dd8aad76b522a60a52fee9cc56d3e46fc3c9a5 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/visitor-plugin-common |
7.0.0-alpha-20260105125118-f6dd8aad76b522a60a52fee9cc56d3e46fc3c9a5 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-document-nodes |
5.0.5-alpha-20260105125118-f6dd8aad76b522a60a52fee9cc56d3e46fc3c9a5 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/gql-tag-operations |
5.0.6-alpha-20260105125118-f6dd8aad76b522a60a52fee9cc56d3e46fc3c9a5 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-operations |
6.0.0-alpha-20260105125118-f6dd8aad76b522a60a52fee9cc56d3e46fc3c9a5 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript-resolvers |
6.0.0-alpha-20260105125118-f6dd8aad76b522a60a52fee9cc56d3e46fc3c9a5 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typed-document-node |
6.1.3-alpha-20260105125118-f6dd8aad76b522a60a52fee9cc56d3e46fc3c9a5 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/typescript |
6.0.0-alpha-20260105125118-f6dd8aad76b522a60a52fee9cc56d3e46fc3c9a5 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/client-preset |
6.0.0-alpha-20260105125118-f6dd8aad76b522a60a52fee9cc56d3e46fc3c9a5 |
npm ↗︎ unpkg ↗︎ |
@graphql-codegen/graphql-modules-preset |
5.0.6-alpha-20260105125118-f6dd8aad76b522a60a52fee9cc56d3e46fc3c9a5 |
npm ↗︎ unpkg ↗︎ |
| function getRootType(operation: OperationTypeNode, schema: GraphQLSchema) { | ||
| switch (operation) { | ||
| case 'query': | ||
| return schema.getQueryType(); | ||
| case 'mutation': | ||
| return schema.getMutationType(); | ||
| case 'subscription': | ||
| return schema.getSubscriptionType(); | ||
| } | ||
| throw new Error(`Unknown operation type: ${operation}`); | ||
| } |
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.
I drived-by moved this function here to make the first bit of this file the related to visitor. This is done fairly consistently across the codebase now to help readability and navigation
| enumPrefix: getConfigValue(rawConfig.enumPrefix, true), | ||
| enumSuffix: getConfigValue(rawConfig.enumSuffix, true), |
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.
As mentioned in the description. This is moved to base-visitor since all 3 children visitors need it.
| enumPrefix: boolean; | ||
| enumSuffix: boolean; |
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.
typescript-operations (the main user of this file) now coerce null -> false, so we don't have to handle null case here.
| // | ||
| // Also use fragment hashing if skipTypename is true, since we | ||
| // then don't have a typename for naming the fragment. |
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.
This is no longer relevant as we no longer have skipTypename for client use cases.
| * ``` | ||
| */ | ||
| noExport?: boolean; | ||
| globalNamespace?: boolean; |
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.
This is already set in base-documents-visitor so we don't have to re-declare it here.
| const processor = new PreResolveTypesProcessor(processorConfig); | ||
|
|
||
| this.setSelectionSetHandler( | ||
| new SelectionSetToObject( | ||
| processor, |
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.
Inlining the processor here, since we no longer have the complicated ternary.
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.
Most test changes are related to skipTypename being a config option that no longer works for client use cases.
Note that since we inlined the config option in the last PR, we start to see TypeScript complaining about unexpected config options. This makes cleanups like this a lot easier.
| throw new Error(`Unknown operation type: ${operation}`); | ||
| } | ||
|
|
||
| export interface ParsedDocumentsConfig extends ParsedTypesConfig { |
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.
Previously, documents-visitor extends types-visitor config types, however, BaseDocumentsVisitor extends BaseVisitor, instead of BaseTypesVisitor.
This may have created some confusion (at least for me) of which option is used for which use case. This change correctly reflects the inheritance path for the visitors.
* Refactor utility function to match practice * Fix config option inheritance - addTypename/skipTypename: is only a types-visitor concern. This is moved to types-visitor from base-visitor - nonOptionalTypename: is a documents-visitor and types-visitor concern. Moved from base-visitor there - extractAllFieldsToTypes: is a documents-visitor concern. Moved from base-visitor there - enumPrefix and enumSuffix: need to be in base-visitor as all 3 types of visitors need this to correctly sync the enum type names. This is moved to base visitor - ignoreEnumValuesFromSchema: is a documents-visitor and types-visitor concern. Moved from base-visitor there. - globalNamespace: is a documents-visitor concern. Moved from base-visitor there Other changes: - documents-visitor no longer extends types-visitor _option types_ as they have two distinct usages now. The types now extend base-visitor types. This is now consistent with documents-visitor extending base-visitor - Classes now handle config parsing and types at the same level e.g. if typescript-operations plugin parses configOne, then the types for configOne must be in that class, rather than in base-documents-visitor * Remove references and tests related to skipTypename in operations plugin * Add changeset
* Refactor utility function to match practice * Fix config option inheritance - addTypename/skipTypename: is only a types-visitor concern. This is moved to types-visitor from base-visitor - nonOptionalTypename: is a documents-visitor and types-visitor concern. Moved from base-visitor there - extractAllFieldsToTypes: is a documents-visitor concern. Moved from base-visitor there - enumPrefix and enumSuffix: need to be in base-visitor as all 3 types of visitors need this to correctly sync the enum type names. This is moved to base visitor - ignoreEnumValuesFromSchema: is a documents-visitor and types-visitor concern. Moved from base-visitor there. - globalNamespace: is a documents-visitor concern. Moved from base-visitor there Other changes: - documents-visitor no longer extends types-visitor _option types_ as they have two distinct usages now. The types now extend base-visitor types. This is now consistent with documents-visitor extending base-visitor - Classes now handle config parsing and types at the same level e.g. if typescript-operations plugin parses configOne, then the types for configOne must be in that class, rather than in base-documents-visitor * Remove references and tests related to skipTypename in operations plugin * Add changeset
Description
This PR re-adjusts a few config options that sits on different visitors incorrectly.
We have 4 main types of visitors:
Note: documents-visitor includes
base-documents-visitorandtypscript-operationsvisitor. I've rolled these two up into one type here for simplicityThe updated config options are:
skipTypename(turned intoaddTypenameinternally): previously lives in the base-visitor. However, it is only a types-visitor concern. documents-visitor has its own way to enforce typename, and resolvers-visitor has its own .resolversTypenamealready.nonOptionalTypename: previously lives in the base-visitor. However, it is a documents-visitor and types-visitor concern.extractAllFieldsToTypes: is a documents-visitor concern as it is only used to generate operation typesenumPrefixandenumSuffix: need to be in base-visitor as all 3 types of visitors need this to correctly sync the enum type names.ignoreEnumValuesFromSchema: is a documents-visitor and types-visitor concern.globalNamespace: is a documents-visitor concern.Other refactors:
Related #10496
Type of change
How Has This Been Tested?