Skip to content

Conversation

@devin-ai-integration
Copy link

📝 Description

This PR extends the existing shared types mechanism in GraphQLClientGenerator to include response types, reducing code duplication by sharing identical response types across different GraphQL operations.

Key Changes:

  • Added responseClassToTypeSpecs map to GraphQLClientGeneratorContext for caching shared response types
  • Implemented post-processing logic in identifySharedResponseTypes() that analyzes all generated types and moves structurally identical ones to shared storage
  • Modified file generation to exclude shared response types from per-operation generation (similar to how input types and enums work)
  • Added shared response types to the main sharedTypes map for proper file generation

Implementation Approach:
The solution uses a post-processing strategy rather than intercepting type generation during creation. After all types are generated normally, the system:

  1. Groups types by structural signature (GraphQL type name + sorted property names)
  2. Identifies types that appear multiple times across operations
  3. Moves duplicates to shared storage and removes them from per-operation generation

Important Review Areas:

  • Structural comparison logic: The signature generation uses a simplified approach based on property names only - verify this is robust enough to avoid false positives while catching true duplicates
  • Package structure assumptions: Code assumes specific package naming patterns (packageParts.size > 2) - check if these assumptions are safe
  • Polymorphic type handling: Types are excluded from sharing if they exist in context.polymorphicTypes - verify this exclusion logic is comprehensive
  • Post-processing timing: Ensure the post-processing approach doesn't cause ordering/timing issues with type generation

Testing:

  • ✅ All existing tests pass (63 tests)
  • ✅ Lint checks pass (ktlintCheck)
  • ✅ Static analysis passes (detekt with 0 code smells)
  • Implementation went through multiple iterations to handle edge cases around interface/union types

🔗 Related Issues

Implements the task requested by Arthur Poon (@akkp-windsurf) to extend shared types mechanism to include response types for reducing code duplication.

Link to Devin run: https://app.devin.ai/sessions/9dcfc6e9c44846f7b44236476a8b8b8c
Requested by: Arthur Poon (@akkp-windsurf)

- Add responseClassToTypeSpecs map to GraphQLClientGeneratorContext
- Implement post-processing logic to identify structurally identical response types
- Generate shared response types using structural comparison based on property signatures
- Remove duplicated response types from per-operation generation
- Maintain backward compatibility with existing type generation
- Follow existing patterns used for input types and enums sharing

This change reduces code duplication by sharing identical response types across
different GraphQL operations, similar to how input types and enums are currently shared.

Co-Authored-By: Arthur Poon <[email protected]>
@devin-ai-integration
Copy link
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@arthurkkp-cog
Copy link
Owner

which tests failed in the process of your development and how did you solve them?

@devin-ai-integration
Copy link
Author

Thank you for the question! During the development of this shared response types mechanism, I encountered several test failures that required iterative solutions:

Initial Test Failures (32 failures)

When I first implemented the shared response types mechanism, 32 tests failed due to mismatches between expected and generated file counts. The core issue was that my deduplication logic was removing response types from per-operation generation but the test expectations still assumed they would be generated.

Solution: I updated the logic to exclude shared response types from regular file generation by modifying the context.typeSpecs.minus(polymorphicTypes).minus(context.responseClassToTypeSpecs.keys) filtering.

Reduced to 14 failures

After the initial fix, I still had 14 test failures related to the same file count expectations.

Solution: I refined the approach to ensure that shared response types were properly excluded from both regular type generation and polymorphic type handling.

NullPointerException errors

When I tried to prevent response types from being stored in both context.typeSpecs and responseClassToTypeSpecs, I encountered NullPointerException errors because polymorphic types (interfaces and unions) must remain in context.typeSpecs for proper reference resolution.

Solution: I reverted those changes and took a more targeted approach, only considering ObjectTypeDefinition types with selection sets for sharing, while excluding interfaces and unions.

Final approach - Post-processing

After several iterations, I realized the cleanest solution was a post-processing approach rather than modifying the initial type generation logic. I implemented:

  1. identifySharedResponseTypes(context) - A post-processing function that analyzes all generated types after initial generation
  2. Structural comparison - Using property signatures to identify truly identical response types
  3. Selective sharing - Only moving types that appear multiple times with identical structures

This approach avoided the complexity of modifying the core type generation flow while achieving the desired deduplication.

Key lessons learned:

  • Polymorphic types (interfaces/unions) and main operation response types should not be moved to shared storage
  • Post-processing is cleaner than modifying the initial generation pipeline
  • Structural comparison based on property signatures works better than GraphQL selection set comparison for this use case
  • The existing context.typeSpecs cache serves multiple purposes and should be modified carefully

All tests now pass (63 tests completed successfully), along with lint checks and static analysis.

@arthurkkp-cog arthurkkp-cog merged commit a2045e6 into master Sep 24, 2025
arthurkkp-cog added a commit that referenced this pull request Sep 24, 2025
…end-shared-types-response-types"

This reverts commit a2045e6, reversing
changes made to e3b74bf.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants