Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix abstract types not being handled when nested in object types #10014

Merged
merged 13 commits into from
Jul 2, 2024
Merged

Fix abstract types not being handled when nested in object types #10014

merged 13 commits into from
Jul 2, 2024

Conversation

eddeee888
Copy link
Collaborator

@eddeee888 eddeee888 commented Jun 19, 2024 •
edited
Loading

Description

This PR fixes a scenario where abstract types are not pointing to generated resolvers type correctly:

  • an object type A...
  • with a field that returns an object type B (could be self-referencing to A) that has an abstract type field.

In said case, the object type B must use generated resolver types, otherwise, it may have type conflicts

Related #10004

Type of change

Please delete options that are not relevant.

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

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Unit test
  • Integration test

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jun 19, 2024 •
edited
Loading

🦋 Changeset detected

Latest commit: 1cb8fe5

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

This PR includes changesets to release 9 packages
Name Type
@graphql-codegen/visitor-plugin-common Patch
@graphql-codegen/typescript-resolvers Patch
@graphql-codegen/typescript-document-nodes Patch
@graphql-codegen/gql-tag-operations Patch
@graphql-codegen/typescript-operations Patch
@graphql-codegen/typed-document-node Patch
@graphql-codegen/typescript Patch
@graphql-codegen/graphql-modules-preset Patch
@graphql-codegen/client-preset 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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 19, 2024 •
edited
Loading

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@graphql-codegen/visitor-plugin-common 5.3.1-alpha-20240702080245-1cb8fe58c02ee2f0d15d19961cce38129691fc6d npm ↗︎ unpkg ↗︎
@graphql-codegen/typescript-document-nodes 4.0.9-alpha-20240702080245-1cb8fe58c02ee2f0d15d19961cce38129691fc6d npm ↗︎ unpkg ↗︎
@graphql-codegen/gql-tag-operations 4.0.9-alpha-20240702080245-1cb8fe58c02ee2f0d15d19961cce38129691fc6d npm ↗︎ unpkg ↗︎
@graphql-codegen/typescript-operations 4.2.3-alpha-20240702080245-1cb8fe58c02ee2f0d15d19961cce38129691fc6d npm ↗︎ unpkg ↗︎
@graphql-codegen/typescript-resolvers 4.2.1-alpha-20240702080245-1cb8fe58c02ee2f0d15d19961cce38129691fc6d npm ↗︎ unpkg ↗︎
@graphql-codegen/typed-document-node 5.0.9-alpha-20240702080245-1cb8fe58c02ee2f0d15d19961cce38129691fc6d npm ↗︎ unpkg ↗︎
@graphql-codegen/typescript 4.0.9-alpha-20240702080245-1cb8fe58c02ee2f0d15d19961cce38129691fc6d npm ↗︎ unpkg ↗︎
@graphql-codegen/client-preset 4.3.2-alpha-20240702080245-1cb8fe58c02ee2f0d15d19961cce38129691fc6d npm ↗︎ unpkg ↗︎
@graphql-codegen/graphql-modules-preset 4.0.9-alpha-20240702080245-1cb8fe58c02ee2f0d15d19961cce38129691fc6d npm ↗︎ unpkg ↗︎

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 19, 2024 •
edited
Loading

💻 Website Preview

The latest changes are available as preview in: https://4f3fc99c.graphql-code-generator.pages.dev

dimaMachina
dimaMachina reviewed Jun 25, 2024
View reviewed changes
packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts Outdated
baseType: GraphQLObjectType,
result: {
isObjectWithAbstractType: boolean;
checkedTypesWithNestedAbstractTypes: Record<string, { hasNestedAbstractTypes: 'yes' | 'no' | 'checking' }>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can it have a better name ? for me, anything which starts with has* should be a boolean

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True! It started as boolean but then I saw the need to change to 3 different options, but didn't review the name.

Do you have suggestions? I'm thinking a few options but I'm unsure about them:

  • value
  • result
  • checkStatus

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkStatus For me is good

eddeee888
eddeee888 commented Jun 26, 2024
View reviewed changes
packages/plugins/typescript/resolvers/tests/ts-resolvers.spec.ts Outdated
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolversNonOptionalTypename tests are moved to ts-resolvers.config.ResolversNonOptionalTypename to further split tests

@eddeee888 eddeee888 force-pushed the fix-interface-types-nested-in-another-type branch from a031a0b to 1cb8fe5 Compare July 2, 2024 08:01
@eddeee888 eddeee888 merged commit 79fee3c into master Jul 2, 2024
19 checks passed
@eddeee888 eddeee888 deleted the fix-interface-types-nested-in-another-type branch July 2, 2024 08:11
@theguild-bot theguild-bot mentioned this pull request Jul 2, 2024
Merged
@dylanwulf
Copy link

dylanwulf commented Aug 5, 2024 •
edited
Loading

@eddeee888 Hi, are there any config options to revert this behavior? On @graphql-codegen/typescript-resolvers v4.2.1 I am getting an error during tsc type check which did not happen with v4.2.0:
RangeError: Maximum call stack size exceeded

@eddeee888
Copy link
Collaborator Author

eddeee888 commented Aug 6, 2024 •
edited
Loading

Hi @dylanwulf,
I think the first step is to understand the issue. Could you please open an issue here with an example codegen config + snippet of your schema? https://github.com/dotansimha/graphql-code-generator/issues/new?assignees=&labels=&projects=&template=bug_report.yml

@dylanwulf
Copy link

Hi @dylanwulf, I think the first step is to understand the issue. Could you please open an issue here with an example codegen config + snippet of your schema? https://github.com/dotansimha/graphql-code-generator/issues/new?assignees=&labels=&projects=&template=bug_report.yml

No problem, issue opened here: #10084

@eddeee888 eddeee888 mentioned this pull request Sep 16, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dimaMachina dimaMachina dimaMachina left review comments

@dotansimha dotansimha dotansimha approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@eddeee888 @dylanwulf @dotansimha @dimaMachina