-
-
Notifications
You must be signed in to change notification settings - Fork 609
Add support for defining fields using Annotated #4059
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 GuideAdds support for defining Strawberry GraphQL fields via typing.Annotated carrying strawberry.field metadata, updates the type resolver to interpret Annotated types (including validation for multiple strawberry.field annotations), wires a new MultipleStrawberryFieldsError into the exception system, and adds tests and docs around this behavior. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4059 +/- ##
===========================================
- Coverage 94.41% 10.96% -83.45%
===========================================
Files 536 537 +1
Lines 35036 35208 +172
Branches 1842 1846 +4
===========================================
- Hits 33079 3862 -29217
- Misses 1659 31203 +29544
+ Partials 298 143 -155 🚀 New features to boost your workflow:
|
4dd1427 to
bbe863a
Compare
|
Thanks for adding the Here's a preview of the changelog: This release adds support for defining fields using the Example usage: from typing import Annotated
import strawberry
@strawberry.type
class Query:
name: Annotated[str, strawberry.field(description="The name")]
age: Annotated[int, strawberry.field(deprecation_reason="Use birthDate instead")]
@strawberry.input
class CreateUserInput:
name: Annotated[str, strawberry.field(description="User's name")]
email: Annotated[str, strawberry.field(description="User's email")]This syntax works alongside the existing assignment syntax: @strawberry.type
class Query:
# Both styles work
field1: Annotated[str, strawberry.field(description="Using Annotated")]
field2: str = strawberry.field(description="Using assignment")All 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.
Hey there - I've reviewed your changes - here's some feedback:
- The new
Annotatedhandling intype_resolver.pyduplicates theStrawberryFieldconstruction logic in three branches; consider extracting a small helper (e.g._build_strawberry_field(field_name, annotation, origin, module, default)) to reduce repetition and keep future changes to field creation centralized. - When a
StrawberryFieldis found inside anAnnotatedtype you currently drop all otherAnnotatedmetadata by using only the first argument as the annotation; if non-Strawberry metadata is expected to be preserved in this case as well, you may want to keep the fullAnnotatedor otherwise explicitly handle or document the loss of the extra metadata.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `Annotated` handling in `type_resolver.py` duplicates the `StrawberryField` construction logic in three branches; consider extracting a small helper (e.g. `_build_strawberry_field(field_name, annotation, origin, module, default)`) to reduce repetition and keep future changes to field creation centralized.
- When a `StrawberryField` is found inside an `Annotated` type you currently drop all other `Annotated` metadata by using only the first argument as the annotation; if non-Strawberry metadata is expected to be preserved in this case as well, you may want to keep the full `Annotated` or otherwise explicitly handle or document the loss of the extra metadata.
## Individual Comments
### Comment 1
<location> `strawberry/types/type_resolver.py:153-162` </location>
<code_context>
+ for arg in rest:
</code_context>
<issue_to_address>
**issue (bug_risk):** Default value from the dataclass field is ignored when a StrawberryField is found in Annotated metadata.
When an `Annotated` type includes a `StrawberryField`, this branch assumes the default is already set on `strawberry_field_from_annotated`. That works only if the user passed `default=` to `strawberry.field()`, but not when the default is defined on the dataclass field itself:
```python
@strawberry.type
class A:
foo: Annotated[int, strawberry.field()] = 5
# or foo: Annotated[int, strawberry.field()] = dataclasses.field(default=5)
```
In these cases, the dataclass `Field` holds the default, so it gets dropped. Previously, `getattr(cls, field.name, dataclasses.MISSING)` preserved this. Please update the Annotated + `StrawberryField` path to also fall back to `getattr(cls, field.name, dataclasses.MISSING)` when the `StrawberryField` default is `dataclasses.MISSING`, so these defaults are not lost.
</issue_to_address>
### Comment 2
<location> `tests/types/test_object_types.py:239` </location>
<code_context>
assert has_object_definition(JSON) is False
+
+
+def test_annotated_field_with_description():
+ @strawberry.type
+ class Query:
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test where `strawberry.field` is not the first metadata in `Annotated` but is still correctly picked up.
All existing `Annotated[...]` tests place `strawberry.field(...)` first in the metadata list, so they don’t verify that order truly doesn’t matter. Please add a case like:
```python
@strawberry.type
class Query:
name: Annotated[str, "meta1", strawberry.field(description="desc"), "meta2"]
definition = get_object_definition(Query)
field = definition.fields[0]
assert field.python_name == "name"
assert field.description == "desc"
assert field.type is str
```
This guards against regressions where the implementation might accidentally depend on `strawberry.field` appearing in a specific position within the `Annotated` metadata.
Suggested implementation:
```python
def test_annotated_field_with_description():
@strawberry.type
class Query:
name: Annotated[str, strawberry.field(description="The name")]
definition = get_object_definition(Query)
field = definition.fields[0]
assert field.python_name == "name"
assert field.description == "The name"
assert field.type is str
def test_annotated_field_with_description_when_field_is_not_first_metadata():
@strawberry.type
class Query:
name: Annotated[
str,
"meta1",
strawberry.field(description="desc"),
"meta2",
]
definition = get_object_definition(Query)
field = definition.fields[0]
assert field.python_name == "name"
assert field.description == "desc"
assert field.type is str
```
Ensure that `Annotated` is imported in this test module, for example:
```python
from typing import Annotated
```
If this import already exists earlier in the file, no further changes are needed.
</issue_to_address>
### Comment 3
<location> `tests/types/test_object_types.py:293` </location>
<code_context>
+ assert field.type == Optional[str]
+
+
+def test_annotated_field_with_default_value():
+ @strawberry.type
+ class Query:
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for defaults/`default_factory` specified inside `strawberry.field(...)` when used in `Annotated`.
The current test only covers a default set via `= "default"` on an `Annotated` field. The new behavior should also support defaults coming from `strawberry.field(...)`, where the default is stored on the `StrawberryField` rather than the class attribute. Please add tests like:
```python
def test_annotated_field_with_default_in_metadata():
@strawberry.type
class Query:
name: Annotated[
str,
strawberry.field(default="default", description="The name"),
]
definition = get_object_definition(Query)
field = definition.fields[0]
assert field.default == "default"
assert Query().name == "default"
def test_annotated_field_with_default_factory_in_metadata():
@strawberry.type
class Query:
tags: Annotated[list[str], strawberry.field(default_factory=list)]
assert Query().tags == []
```
These ensure `StrawberryField` defaults/default_factories are preserved when used via `Annotated`.
</issue_to_address>
### Comment 4
<location> `tests/types/test_object_types.py:416-417` </location>
<code_context>
+ assert "Use birthYear" in schema_str
+
+
+def test_annotated_field_with_input():
+ """Test that Annotated fields work correctly with input types."""
+
</code_context>
<issue_to_address>
**suggestion (testing):** Expand input-type tests to cover optional and defaulted `Annotated` fields.
Consider adding cases for:
- An optional input field, e.g. `name: Annotated[str | None, strawberry.field(description="...")]`, and asserting it is optional in the schema and functions correctly.
- A defaulted input field, e.g. `age: Annotated[int, strawberry.field(description="...", default=18)]`, and asserting the default appears both in the field definition and the GraphQL schema.
This would align input-type coverage with the existing object-type `Annotated` tests.
```suggestion
def test_annotated_field_with_input():
"""Test that Annotated fields work correctly with input types."""
@strawberry.input
class UserInput:
name: Annotated[
str | None,
strawberry.field(description="The user name"),
]
age: Annotated[
int,
strawberry.field(description="The user age", default=18),
]
@strawberry.type
class Query:
@strawberry.field
def get_user_age(self, input: UserInput) -> int:
return input.age
schema = strawberry.Schema(query=Query)
schema_str = str(schema)
# Optional field: should not be non-null in the schema
assert "input UserInput" in schema_str
assert '"""The user name"""' in schema_str
assert 'name: String' in schema_str
assert "name: String!" not in schema_str
# Defaulted field: default should appear in the schema SDL
assert '"""The user age"""' in schema_str
assert "age: Int = 18" in schema_str
# Executing without providing the optional field or the defaulted field value
# should use the default for age and treat name as optional.
result_with_defaults = schema.execute_sync(
"{ getUserAge(input: {}) }",
)
assert result_with_defaults.errors is None
assert result_with_defaults.data == {"getUserAge": 18}
# Executing with an explicit value for age should override the default.
result_with_value = schema.execute_sync(
"{ getUserAge(input: { age: 30 }) }",
)
assert result_with_value.errors is None
assert result_with_value.data == {"getUserAge": 30}
```
</issue_to_address>
### Comment 5
<location> `docs/errors/multiple-strawberry-fields.md:9` </location>
<code_context>
+
+## Description
+
+This error is thrown when using multiple `strawberry.field()` annotations inside
+an `Annotated` type. For example, the following code will throw this error:
+
</code_context>
<issue_to_address>
**suggestion (typo):** Consider using Python-idiomatic terminology by saying the error is "raised" instead of "thrown".
In Python, exceptions are typically described as being "raised" rather than "thrown". Consider updating the wording to: "This error is raised when using multiple `strawberry.field()` annotations inside..." to match common Python terminology.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
bbe863a to
5ffbc43
Compare
Greptile OverviewGreptile SummaryAdded support for defining Strawberry GraphQL fields using Python's Key Changes:
Implementation Details: Potential Concern: Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant Decorator as @strawberry.type
participant Dataclass as dataclasses.dataclass
participant TypeResolver as _get_fields()
participant AnnotatedParser as Annotated Handler
participant Schema as GraphQL Schema
User->>Decorator: Define class with Annotated field
Note over User: name: Annotated[str, strawberry.field(...)]
Decorator->>Dataclass: Wrap class as dataclass
Dataclass->>Dataclass: Process fields and create __init__
Note over Dataclass: Creates Field objects with defaults
Decorator->>TypeResolver: Extract Strawberry fields
TypeResolver->>TypeResolver: Iterate dataclasses.fields(cls)
alt Field is StrawberryField
TypeResolver->>TypeResolver: Validate and process existing field
else Field is regular dataclass Field
TypeResolver->>AnnotatedParser: Check if type is Annotated
alt Type is Annotated with StrawberryField
AnnotatedParser->>AnnotatedParser: Extract StrawberryField from metadata
AnnotatedParser->>AnnotatedParser: Check for multiple fields (raises error)
AnnotatedParser->>AnnotatedParser: Set python_name, type_annotation, origin
AnnotatedParser->>TypeResolver: Return configured StrawberryField
else Type is Annotated without StrawberryField
AnnotatedParser->>TypeResolver: Create basic StrawberryField
else Type is not Annotated
TypeResolver->>TypeResolver: Create basic StrawberryField
end
end
TypeResolver->>Schema: Return list of StrawberryFields
Schema->>Schema: Build GraphQL schema definition
|
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.
5 files reviewed, 1 comment
CodSpeed Performance ReportMerging #4059 will not alter performanceComparing Summary
|
- Fix terminology: use "raised" instead of "thrown" in docs - Transfer default values from dataclass field to StrawberryField for Annotated syntax - Add test for input type defaults appearing in GraphQL schema
| field_type = field.type | ||
| strawberry_field_from_annotated: StrawberryField | None = None | ||
|
|
||
| if get_origin(field_type) is Annotated: |
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: maybe instead of this, implement a separate function, like _get_field_from_annotated(), which receives the field, does the validation we have internally here, and then we have a single place creating a StrawberryField instead of 2
Co-authored-by: Thiago Bellini Ribeiro <thiago@bellini.dev>
for more information, see https://pre-commit.ci

Summary by Sourcery
Add support for configuring Strawberry fields via typing.Annotated metadata and surface a clear error when multiple field configurations are provided.
New Features:
Enhancements:
Documentation:
Tests:
Chores: