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.

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

fix: Fix an issue preventing typing root with strawberry.Parent #3529

Merged
merged 2 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
Release type: patch

This release fixes a typing issue where trying to type a `root` argument with
`strawberry.Parent` would fail, like in the following example:

```python
import strawberry


@strawberry.type
class SomeType:
@strawberry.field
def hello(self, root: strawberry.Parent[str]) -> str:
return "world"
```

This should now work as intended.
31 changes: 13 additions & 18 deletions strawberry/types/fields/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,30 +231,25 @@ def reserved_parameters(
@cached_property
def arguments(self) -> List[StrawberryArgument]:
"""Resolver arguments exposed in the GraphQL Schema."""
parameters = self.signature.parameters.values()
reserved_parameters = set(self.reserved_parameters.values())
populated_reserved_parameters = set(
key for key, value in self.reserved_parameters.items() if value is not None
)
root_parameter = self.reserved_parameters.get(ROOT_PARAMSPEC)
patrick91 marked this conversation as resolved.
Show resolved Hide resolved
parent_parameter = self.reserved_parameters.get(PARENT_PARAMSPEC)
patrick91 marked this conversation as resolved.
Show resolved Hide resolved

# TODO: Maybe use SELF_PARAMSPEC in the future? Right now
patrick91 marked this conversation as resolved.
Show resolved Hide resolved
# it would prevent some common pattern for integrations
# (e.g. django) of typing the `root` parameters as the
# type of the real object being used
if (
conflicting_arguments := (
populated_reserved_parameters
# TODO: Maybe use SELF_PARAMSPEC in the future? Right now
# it would prevent some common pattern for integrations
# (e.g. django) of typing the `root` parameters as the
# type of the real object being used
& {ROOT_PARAMSPEC, PARENT_PARAMSPEC}
)
) and len(conflicting_arguments) > 1:
root_parameter is not None
and parent_parameter is not None
and root_parameter.name != parent_parameter.name
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Clarify the logic for checking parameter names.

The condition root_parameter.name != parent_parameter.name might benefit from a comment explaining why the names should not match. This will help future maintainers understand the rationale behind this check.

Suggested change
and root_parameter.name != parent_parameter.name
# Ensure the root and parent parameters do not refer to the same argument
and root_parameter.name != parent_parameter.name

):
raise ConflictingArgumentsError(
self,
[
cast(Parameter, self.reserved_parameters[key]).name
for key in conflicting_arguments
],
[root_parameter.name, parent_parameter.name],
)

parameters = self.signature.parameters.values()
reserved_parameters = set(self.reserved_parameters.values())
missing_annotations: List[str] = []
arguments: List[StrawberryArgument] = []
user_parameters = (p for p in parameters if p not in reserved_parameters)
Expand Down
83 changes: 82 additions & 1 deletion tests/fields/test_resolvers.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import dataclasses
import textwrap
import types
from typing import ClassVar, List, no_type_check
from typing import Any, ClassVar, List, no_type_check

import pytest

Expand Down Expand Up @@ -453,3 +454,83 @@ def bar(x: str = "bar"):

assert len(parameters_map) == 2
assert len(parameters_set) == 2


def test_annotation_using_parent_annotation():
@strawberry.type
class FruitType:
name: str

@strawberry.field
@staticmethod
def name_from_parent(parent: strawberry.Parent[Any]) -> str:
return f"Using 'parent': {parent.name}"

@strawberry.type
class Query:
@strawberry.field
@staticmethod
def fruit() -> FruitType:
return FruitType(name="Strawberry")

schema = strawberry.Schema(query=Query)
expected = """\
type FruitType {
name: String!
nameFromParent: String!
}

type Query {
fruit: FruitType!
}
"""

assert textwrap.dedent(str(schema)) == textwrap.dedent(expected).strip()

result = schema.execute_sync("query { fruit { name nameFromParent } }")
assert result.data == {
"fruit": {
"name": "Strawberry",
"nameFromParent": "Using 'parent': Strawberry",
}
}


def test_annotation_using_parent_annotation_but_named_root():
@strawberry.type
class FruitType:
name: str

@strawberry.field
@staticmethod
def name_from_parent(root: strawberry.Parent[Any]) -> str:
return f"Using 'root': {root.name}"

@strawberry.type
class Query:
@strawberry.field
@staticmethod
def fruit() -> FruitType:
return FruitType(name="Strawberry")

schema = strawberry.Schema(query=Query)
expected = """\
type FruitType {
name: String!
nameFromParent: String!
}

type Query {
fruit: FruitType!
}
"""

assert textwrap.dedent(str(schema)) == textwrap.dedent(expected).strip()

result = schema.execute_sync("query { fruit { name nameFromParent } }")
assert result.data == {
"fruit": {
"name": "Strawberry",
"nameFromParent": "Using 'root': Strawberry",
}
}
Loading