graphql-code-generator icon indicating copy to clipboard operation
graphql-code-generator copied to clipboard

fix(visitor-plugin-common): Use rhs of 'import as' statements #9100

Open Otard95 opened this issue 2 years ago • 2 comments

🚨 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:

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.

Otard95 avatar Aug 15 '23 09:08 Otard95

⚠️ 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

changeset-bot[bot] avatar Aug 15 '23 09:08 changeset-bot[bot]

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.

Otard95 avatar Aug 15 '23 13:08 Otard95