fix(visitor-plugin-common): Use rhs of 'import as' statements #9100
🚨 IMPORTANT: Please do not create a Pull Request without creating an issue first.
Any change needs to be discussed before proceeding. Failure to do so may result in the rejection of the pull request.
Description
This PR is created in response to contribution guides "A Typical Contributor Workflow" stage/2-failing-test. As such, this change is for the moment just a failing test that reflects the issue.
EDIT: Implemented possible solution as described in https://github.com/dotansimha/graphql-code-generator/pull/9592#issuecomment-1678919861
This PR is related to the #9100 issue.
Short recap of the issue
When the TypeScript enum and the GraphQL enum aren't named identically the generated resolver types are invalid and will not compile.
With the following config
{
"enumValues": {
"MyGqlEnum": "@swydo/foo#MyTsEnum"
}
}
the following is generated:
import { MyTsEnum as MyGqlEnum } from '@swydo/collections';
export type ResolversTypes = {
MyGqlEnum: MyTsEnum;
}
Type of change
- [x] Bug fix (non-breaking change which fixes an issue)
Screenshots/Sandbox
Sandbox provided by @quintstoffers in the original issue: https://codesandbox.io/s/graphql-codegen-enum-types-wt98s9?file=/resolvers.ts
I've also created a repo reproducing the issue: https://github.com/Otard95/graphql-code-generator-issue-reproduction
How Has This Been Tested?
As described by stage/2-failing-test at this point this is just a draft PR with a failing test.
- [x] TypeScript Resolvers Plugin › issues › #9100 - should use the rhs of 'import as' when using external enum with different name
Test Environment:
- OS: Linux 6.1.0-11-amd64 Debian 6.1.38-4 (2023-08-08) x86_64 GNU/Linux
-
[email protected] -
@graphql-codegen/[email protected] -
@graphql-codegen/[email protected] -
@graphql-codegen/[email protected] - NodeJS: v18.15.0
Checklist:
- [x] I have followed the CONTRIBUTING doc and the style guidelines of this project
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published in downstream modules
- Not sure if this is necessary as the change is in a single package, and I don't know if I'd need to manually update the dependency version in other packages, or if this is automated.
Further comments
I'm unsure how much time I can devote to this, but I hope this could at the very least help move this issue forward and spark some discussion.
⚠️ No Changeset found
Latest commit: c9bcee98de37282ad195ba0f74dd57be30f09444
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
One option to solve this could be updating https://github.com/dotansimha/graphql-code-generator/blob/077f7bff8d5fd723b87204b47c8bf096aa72ade5/packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts#L831 to something like:
} else if (isEnumType(schemaType) && this.config.enumValues[typeName]) {
const sourceIdentifier = this.config.enumValues[typeName].sourceIdentifier;
const typeIdentifier = this.config.enumValues[typeName].typeIdentifier;
const importIdentifier = this.config.enumValues[typeName].importIdentifier;
// In `packages/plugins/other/visitor-plugin-common/src/base-types-visitor.ts:847`
// (https://github.com/dotansimha/graphql-code-generator/blob/077f7bff8d5fd723b87204b47c8bf096aa72ade5/packages/plugins/other/visitor-plugin-common/src/base-types-visitor.ts#L847)
// the import statement is generated as an 'import as' statement given
// this condition. In this case the `sourceIdentifier` is renamed to the
// `typeIdentifier` in the import statement, and therefore no longer
// usable as a identifier. We should instead use the `typeIdentifier`.
const isAsImport = importIdentifier === sourceIdentifier && sourceIdentifier !== typeIdentifier;
prev[typeName] = isAsImport
? this.config.enumValues[typeName].typeIdentifier
: this.config.enumValues[typeName].sourceIdentifier ||
this.convertName(this.config.enumValues[typeName].typeIdentifier);
} else if (hasDefaultMapper && !hasPlaceholder(this.config.defaultMapper.type)) {
With this change, all tests including my new one passes. One downside is that it assumes knowledge of a separate part of the tool, which may not be desirable. But as of now I don't see another way, that said I'm new to the codebase.