Fix/map is not a function

[mariusmuntean]

Description

upgraded dependencies and fixed broken code - mostly emulated other plugins.

Related #92

Type of change

Please delete options that are not relevant.

• [x] Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

No additional tests necessary.

Test Environment:

• OS: macOS
• @graphql-codegen/c-sharp: 5.0.0
• @graphql-codegen/c-sharp-common: 1.0.0
• • @graphql-codegen/c-sharp-operations: 3.0.0
• NodeJS: v16.13.2

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] I have made corresponding changes to the documentation
• [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
• [x] Any dependent changes have been merged and published in downstream modules

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: ab099c0987c298b06d7e68f1e64c77d8be9b6968

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@graphql-codegen/c-sharp-operations	Patch
@graphql-codegen/c-sharp-common	Patch
@graphql-codegen/c-sharp	Patch

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

[mariusmuntean]

@saihaj I gave it a go. The issue seems to have been in CSharpOperationsVisitor._gql where a OperationDefinitionNode was passed to _transformFragments() instead of a string[]. I'm still not very familiar with the code and I can't explain why the tests didn't catch this. After upgrading the dependencies of the c-sharp-operations plugin, the error became apparent and the fix was fairly straightforward.

[mariusmuntean]

The existing tests already check the correct output.

The CSharp visitor inherits from base visitor which is not from the community repo, but the main repo.

For some reason, the base visitor is different when running the tests from the visitor that is available at runtime. The one that is available at runtime seems to be the one from the latest package version, where some method signatures have changed.

Unfortunately my JavaScript foo is not enough to understand why there's this discrepancy. I'm guessing it must be a config issue in the community package.

[saihaj]

I meant test more for the case where we could repro this .map function issue

[mariusmuntean]

I didn't find find any and could only observe the issue while debugging. Today I have a little time and I want to give it another go.

[mariusmuntean]

I think I identified the cause of this. The plugin @graphql-codegen/c-sharp-operations has this dependency "@graphql-codegen/visitor-plugin-common": "^2.12.1". In a project that uses this plugin, the @graphql-codegen/visitor-plugin-common version that is installed is 2.13.8, i.e. the latest version that is compatible with the requested one ^2.12.1 (due to the caret, newer minor and patch versions are allowed).

Now the problem is that in dotansimha/graphql-code-generator commit 4f290aa7279a05ffa40920c1c9e5e5b37c164335 (link) the ClientSideBaseVisitor_transformFragments(...) method was changed in a breaking way, from:

protected _transformFragments(document: FragmentDefinitionNode | OperationDefinitionNode): string[] {
  const includeNestedFragments =
    this.config.documentMode === DocumentMode.documentNode ||
    (this.config.dedupeFragments && document.kind === 'OperationDefinition');

  return this._extractFragments(document, includeNestedFragments).map(document =>
    this.getFragmentVariableName(document)
  );
}

to:

protected _transformFragments(fragmentNames: Array<string>): string[] {
  return fragmentNames.map(document => this.getFragmentVariableName(document));
}

Notice that the new function signature takes an array, on which it immediately calls map(). This is the "map" from the error message. The CSharpOperationsVisitor._gql(...) simply passes the wrong argument and it fails.

The changeset in that commit is of type patch, but it should have been major.

A way to reproduce this in the source is to explicitely define a dependency on "@graphql-codegen/visitor-plugin-common": "2.13.8" - all the tests will fail with the "map is not a function" error

What I still don't know is why is version 2.13.8 not installed when running the tests, but it is installed when using the plugin in a project to generate sources? Maybe someone with more JavaScript foo can help me out here so that we can make such issues visible in code, not at runtime. UPDATE 22.08.2024: answering my own question here. 2.13.8 is not installed when running the test because of the yarn.lock where 2.13.7 is pinned.

Coming back to this PR, I updated the dependency version of visitor-plugin-common to the latest version and I fixed the breaking changes. In my opinion this should be enough to get the plugin running again.

[mariusmuntean]

@saihaj did you have a chance to look at my findings? What do you think?

[saihaj]

@saihaj did you have a chance to look at my findings? What do you think?

will try to check it out this week

[mariusmuntean]

@saihaj any chance to look at the PR?

[mariusmuntean]

I'm really glad you merged the PR. What's the release schedule for this plugin? I saw that quite a few people were affected by this bug and they'd profit from a fixed package version. I also plan to write a Medium post about this plugin, but I can only publish it once the fixed version is released.

[saihaj]

released @graphql-codegen/[email protected], @graphql-codegen/[email protected], @graphql-codegen/[email protected]

[saihaj]

I also plan to write a Medium post about this plugin, but I can only publish it once the fixed version is released.

That would be awesome! If you want to co-post on our Guild blog that would be great too.
https://github.com/the-guild-org/website/tree/master/website/pages/blog