strawberry icon indicating copy to clipboard operation
strawberry copied to clipboard

Take oneOf directive into account in codegen module

Open enoua5 opened this issue 1 year ago • 10 comments

Description

Adds support for oneOf to the codegen module

  • The codegen manager now passes is_one_of to GraphQLObjectType making it part of the codegen plugin api
  • Python plugin has been updated to output oneOf types as a union of classes with one field each
  • Typescript plugin has been updated to output oneOf types as a union of objects where in each all but one field has a type of never

Types of Changes

  • [ ] Core
  • [ ] Bugfix
  • [x] New feature
  • [x] Enhancement/optimization
  • [ ] Documentation

Issues Fixed or Closed by This PR

  • None

Checklist

  • [x] My code follows the code style of this project.
  • [ ] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [x] I have read the CONTRIBUTING document.
  • [x] I have added tests to cover my changes.
  • [x] I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Summary by Sourcery

Implement support for the oneOf directive in the code generation module, enhancing the Python and TypeScript plugins to handle oneOf types as unions. Add corresponding tests and snapshots to verify the new functionality.

New Features:

  • Add support for the oneOf directive in the code generation module, allowing the generation of union types for oneOf fields in both Python and TypeScript plugins.

Enhancements:

  • Update the Python plugin to output oneOf types as a union of classes, each with one field.
  • Update the TypeScript plugin to output oneOf types as a union of objects, where each object has all but one field typed as never.

Tests:

  • Add new test cases and snapshots for the oneOf directive in both Python and TypeScript code generation to ensure correct functionality.

enoua5 avatar Sep 28 '24 00:09 enoua5

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.83%. Comparing base (92ae942) to head (1347bde). Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3652      +/-   ##
==========================================
+ Coverage   96.76%   96.83%   +0.06%     
==========================================
  Files         522      503      -19     
  Lines       33824    33457     -367     
  Branches     5635     5596      -39     
==========================================
- Hits        32731    32397     -334     
+ Misses        863      830      -33     
  Partials      230      230              

codecov[bot] avatar Sep 28 '24 00:09 codecov[bot]

CodSpeed Performance Report

Merging #3652 will not alter performance

Comparing enoua5:codegen-oneof (1347bde) with main (3484480)

Summary

✅ 15 untouched benchmarks

codspeed-hq[bot] avatar Sep 28 '24 00:09 codspeed-hq[bot]

Reviewer's Guide by Sourcery

This pull request adds support for the oneOf directive in the codegen module. The changes primarily affect the Python and TypeScript plugins, introducing new logic to handle oneOf types in code generation. The implementation includes updates to object type handling, field printing, and type definitions to accommodate the oneOf functionality.

Sequence Diagram

sequenceDiagram
    participant CG as Codegen Manager
    participant PY as Python Plugin
    participant TS as TypeScript Plugin
    CG->>CG: Set is_one_of flag in GraphQLObjectType
    CG->>PY: Process oneOf type
    PY->>PY: Generate union of classes
    CG->>TS: Process oneOf type
    TS->>TS: Generate union of objects

File-Level Changes

Change Details Files
Added support for oneOf directive in the codegen module
  • Updated GraphQLObjectType to include is_one_of flag
  • Modified _collect_type_from_strawberry_type to set is_one_of flag
  • Added new methods to handle oneOf object types in Python and TypeScript plugins
  • Updated existing methods to account for oneOf types
strawberry/codegen/query_codegen.py
strawberry/codegen/types.py
strawberry/codegen/plugins/python.py
strawberry/codegen/plugins/typescript.py
Updated Python plugin to generate oneOf types as a union of classes
  • Added _print_oneof_object_type method to generate oneOf classes
  • Modified _print_field method to handle oneOf members
  • Updated _print_type to use oneOf-specific logic when applicable
strawberry/codegen/plugins/python.py
Updated TypeScript plugin to generate oneOf types as a union of objects
  • Added _print_oneof_object_type method to generate oneOf types
  • Added _print_oneof_field method to handle oneOf fields
  • Updated _print_type to use oneOf-specific logic when applicable
strawberry/codegen/plugins/typescript.py
Added test cases and snapshots for oneOf functionality
  • Created new test queries for oneOf types
  • Added snapshot files for Python and TypeScript oneOf output
tests/codegen/conftest.py
tests/codegen/snapshots/typescript/oneof_typename.ts
tests/codegen/snapshots/python/oneof_typename.py
tests/codegen/snapshots/typescript/oneof.ts
tests/codegen/snapshots/python/oneof.py
tests/codegen/queries/oneof_typename.graphql
tests/codegen/queries/oneof.graphql

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an issue from a review comment by replying to it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

  • Contact our support team for questions or feedback.
  • Visit our documentation for detailed guides and information.
  • Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai[bot] avatar Sep 28 '24 02:09 sourcery-ai[bot]

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


The Query Codegen system now supports the @oneOf directive.

When writing plugins, you can now access GraphQLObjectType.is_one_of to determine if the object being worked with has a @oneOf directive.

The default plugins have been updated to take advantage of the new attribute.

For example, given this schema:

@strawberry.input(one_of=True)
class OneOfInput:
    a: Optional[str] = strawberry.UNSET
    b: Optional[str] = strawberry.UNSET


@strawberry.type
class Query:
    @strawberry.field
    def one_of(self, value: OneOfInput) -> str: ...


schema = strawberry.Schema(Query)

And this query:

query OneOfTest($value: OneOfInput!) {
  oneOf(value: $value)
}

The query codegen can now generate this Typescript file:

type OneOfTestResult = {
    one_of: string
}

type OneOfInput = { a: string, b?: never }
    | { a?: never, b: string }

type OneOfTestVariables = {
    value: OneOfInput
}

Here's the tweet text:

🆕 Release (next) is out! Thanks to Jacob Allen for the PR 👏

Get it here 👉 https://strawberry.rocks/release/(next)

botberry avatar Sep 28 '24 02:09 botberry

I unchecked "My change requires a change to the documentation". I would add a note about this to the documentation on codegen plugins, but the docs already don't go over any of the other types that are passed into the plugin.

enoua5 avatar Sep 29 '24 05:09 enoua5

If anyone has opinions on how those Typescript unions should be formatted, I'd be happy to take the feedback :smile:

enoua5 avatar Oct 02 '24 02:10 enoua5

@enoua5 I'll check this on the weekend! thank you so much 😊

patrick91 avatar Oct 02 '24 07:10 patrick91

If anyone has opinions on how those Typescript unions should be formatted, I'd be happy to take the feedback 😄

Looking at dotansimha/graphql-code-generator, it looks like it would generate code like this:

type OneOfInput =
  { a: string, b?: never }
  |  { a?: never, b: string };

I'm not a big fan of the hanging =, but, thoughts?

enoua5 avatar Oct 06 '24 20:10 enoua5

@enoua5 I think that's fine, we could maybe in future run prettier/biome to the output if the user has one of them installed :)

patrick91 avatar Oct 06 '24 22:10 patrick91

@patrick91 That's a fair point. Do you think its not worth changing it from how it is now, then?

enoua5 avatar Oct 06 '24 23:10 enoua5