-
-
Notifications
You must be signed in to change notification settings - Fork 602
fix(codegen): Relay Node id field generated as _id instead of id #4053
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.
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's GuideFixes query codegen to use each Strawberry field’s GraphQL name (e.g. Relay Node File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4053 +/- ##
=======================================
Coverage 94.41% 94.41%
=======================================
Files 536 537 +1
Lines 35036 35077 +41
Branches 1842 1842
=======================================
+ Hits 33079 33118 +39
- Misses 1659 1661 +2
Partials 298 298 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #4053 will not alter performanceComparing Summary
|
_id instead of id
_id instead of id|
Thanks for adding the Here's a preview of the changelog: Fixed a bug in the codegen where Relay Node's The codegen now correctly respects the explicit Here's the tweet text: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile OverviewGreptile SummaryThis PR fixes a bug where Relay Node's Key Changes:
Impact: Confidence Score: 5/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User as User Query
participant QC as QueryCodegen
participant Helper as _get_field_name()
participant Schema as Strawberry Schema
participant Field as StrawberryField
User->>QC: Generate code for Relay query
QC->>Schema: Get Relay Node type definition
Schema-->>QC: Node with _id method (@field(name="id"))
QC->>QC: Process field for codegen
QC->>Helper: _get_field_name(field)
Helper->>Field: Check field.graphql_name
alt graphql_name is set (e.g., "id")
Field-->>Helper: "id" (explicit GraphQL name)
Helper-->>QC: Return "id"
else graphql_name is None
Field-->>Helper: None
Helper->>Field: Get field.name (Python name)
Field-->>Helper: Python method name
Helper-->>QC: Return Python name
end
QC->>QC: Create GraphQLField with correct name
QC-->>User: Generated code with "id: str"
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
20 files reviewed, no comments
|
@sourcery-ai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bellini666
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| return strawberry.Schema(query=Query) | ||
|
|
||
|
|
||
| HERE = Path(__file__).parent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: move this to the top of the file, after imports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, fixed!
| RELAY_QUERIES = list((HERE / "relay_queries").glob("*.graphql")) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("plugin_class", "plugin_name", "extension"), | ||
| [ | ||
| (PythonPlugin, "python", "py"), | ||
| (TypeScriptPlugin, "typescript", "ts"), | ||
| ], | ||
| ids=["python", "typescript"], | ||
| ) | ||
| @pytest.mark.parametrize("query", RELAY_QUERIES, ids=[x.name for x in RELAY_QUERIES]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick:
| RELAY_QUERIES = list((HERE / "relay_queries").glob("*.graphql")) | |
| @pytest.mark.parametrize( | |
| ("plugin_class", "plugin_name", "extension"), | |
| [ | |
| (PythonPlugin, "python", "py"), | |
| (TypeScriptPlugin, "typescript", "ts"), | |
| ], | |
| ids=["python", "typescript"], | |
| ) | |
| @pytest.mark.parametrize("query", RELAY_QUERIES, ids=[x.name for x in RELAY_QUERIES]) | |
| @pytest.mark.parametrize( | |
| ("plugin_class", "plugin_name", "extension"), | |
| [ | |
| (PythonPlugin, "python", "py"), | |
| (TypeScriptPlugin, "typescript", "ts"), | |
| ], | |
| ids=["python", "typescript"], | |
| ) | |
| @pytest.mark.parametrize("query", RELAY_QUERIES, ids=[x.name for x in (HERE / "relay_queries").glob("*.graphql")]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @bellini666 , I've updated!


This PR fixes a bug in the codegen where Relay Node's
idfield was incorrectly generated as_idin the output code.Description
The Relay Node class defines its
idfield like this:The codegen was using
field.name(the Python name_id) instead of respecting the explicitgraphql_name(id) set via the decorator.Types of Changes
Issues Fixed or Closed by This PR
Checklist
Summary by Sourcery
Tests:
id: strusing a sample query and schemaSummary by Sourcery
Ensure code generation respects GraphQL field names (e.g. Relay Node
id) instead of Python attribute names.Bug Fixes:
_idinstead of the GraphQL field nameidin codegen outputs.Enhancements:
graphql_namewhen present.Documentation:
idfield name generation in codegen.Tests:
idfields, aliases, fragments, lists, nested nodes, and variables.