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

Fix issue where selection set flattening uses the wrong parent type

Open mvestergaard opened this issue 4 years ago • 5 comments

Description

So let me just start out by saying that I've tried pretty much everything to replicate this problem in a small example. I even took our huge GQL schema into a sandbox and tried writing various queries to replicate the problem, without luck.

The gist of this is basically that after upgrading to the newer versions of the generator, we get this error:

TypeError: Cannot read property 'type' of undefined
        at D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-codegen\visitor-plugin-common\index.js:3027:65 
        at Array.map (<anonymous>)
        at PreResolveTypesProcessor.transformPrimitiveFields (D:\source\roadmap-gh\src\Nordic\UI\node_modules\@grap
hql-codegen\visitor-plugin-common\index.js:3024:23)
        at SelectionSetToObject.buildSelectionSetString (D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-c
odegen\visitor-plugin-common\index.js:2868:32)
        at D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-codegen\visitor-plugin-common\index.js:2759:41 
        at Array.reduce (<anonymous>)
        at SelectionSetToObject._buildGroupedSelections (D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-c
odegen\visitor-plugin-common\index.js:2749:80)
        at SelectionSetToObject.transformSelectionSet (D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-cod
egen\visitor-plugin-common\index.js:2913:54)
        at SelectionSetToObject.buildSelectionSetString (D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-c
odegen\visitor-plugin-common\index.js:2862:89)
        at D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-codegen\visitor-plugin-common\index.js:2759:41 
    TypeError: Cannot read property 'type' of undefined
        at D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-codegen\visitor-plugin-common\index.js:3027:65 
        at Array.map (<anonymous>)
        at PreResolveTypesProcessor.transformPrimitiveFields (D:\source\roadmap-gh\src\Nordic\UI\node_modules\@grap
hql-codegen\visitor-plugin-common\index.js:3024:23)
        at SelectionSetToObject.buildSelectionSetString (D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-c
odegen\visitor-plugin-common\index.js:2868:32)
        at D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-codegen\visitor-plugin-common\index.js:2759:41 
        at Array.reduce (<anonymous>)
        at SelectionSetToObject._buildGroupedSelections (D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-c
odegen\visitor-plugin-common\index.js:2749:80)
        at SelectionSetToObject.transformSelectionSet (D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-cod
egen\visitor-plugin-common\index.js:2913:54)
        at SelectionSetToObject.buildSelectionSetString (D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-c
odegen\visitor-plugin-common\index.js:2862:89)
        at D:\source\roadmap-gh\src\Nordic\UI\node_modules\@graphql-codegen\visitor-plugin-common\index.js:2759:41 

After a ton of debugging, I arrived at this suggested solution. Here are some screenshots to try and explain what is going wrong:

At this point where flattenSelectionSet is called, the block scoped parentSchemaType is ClientPerson, but the class scoped _parentSchemaType is GetClientResult

GetClientResult is a union type consisting of either ClientPerson or ClientCompany or one of a few error types.

image

The problem with this is that it ends up with a flattened list of fields, that contain fields specific to ClientCompany, which it then tries to apply to ClientPerson.

image

Pretty sure this is the same issue reported in #5061

I've also found that setting preResolveTypes to false, and/or inlineFragmentTypes to combine will prevent the issue as well. I think that basically reverts to older behavior, but I assume you changed the defaults for a reason.

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?

It has been tested on our very large schema and code base. As mentioned, I have not found a way to create a small replication of the issue. I have therefore not been able to write a test for it either.

mvestergaard avatar Oct 20 '21 20:10 mvestergaard

⚠️ No Changeset found

Latest commit: 3cdb249fffcd703d1705000eaf699879d9c738e7

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 Oct 20 '21 20:10 changeset-bot[bot]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/theguild/graphql-code-generator/2ETS7kSXQwHG1kwQht6mb9eZ58ZH
✅ Preview: Failed

vercel[bot] avatar Oct 20 '21 20:10 vercel[bot]

Hey @mvestergaard, thank you for debugging this and finding a fix that works for your code-base! Did you manage to track down which operation caused this issue? If we have the schema and operation combination it shouldn't be impossible to solve this 🤔

I wanna merge this, but we really need a test for it!

n1ru4l avatar Nov 05 '21 09:11 n1ru4l

I get that, and unfortunately no.

With this fix, I managed to at least get the types for our code base built. However the types that were built were full of errors, so there are other things blocking us from upgrading as well.

We use some kinda niche features like near-operations-file, and a custom wrapper for the apollo tooling, which seems to be things that break every other release of this project 😢

mvestergaard avatar Nov 05 '21 09:11 mvestergaard

Of course... I can look at it again and try to find the root cause when time permits. I'll change this to a draft in the meantime.

mvestergaard avatar Nov 18 '21 10:11 mvestergaard

#8664 Adds this fix, along with a test, closing this one.

mvestergaard avatar Nov 30 '22 07:11 mvestergaard

https://github.com/dotansimha/graphql-code-generator/releases/tag/release-1669935201062

saihaj avatar Dec 01 '22 22:12 saihaj