Add new strawberry.Parent annotation to support static resolvers with non-self types. #3017

mattalbr · 2023-08-07T23:00:04Z

Description

Allows creating a resolver that depends on a parent which has a different type than the resolver's class, as in self isn't particularly accurate. This is much more descriptive (and allows type checking) than calling the parameter "self" and knowing it will be a different type. See description in #515

Types of Changes

Core
Bugfix
[ x ] New feature
Enhancement/optimization
Documentation

Issues Fixed or Closed by This PR

Should class resolvers always have the `self` argument? #515

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).

Unsure if this needs documentation!

botberry · 2023-08-07T23:02:18Z

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

Adds new strawberry.Parent type annotation to support resolvers without use of self.

E.g.

@dataclass
class UserRow:
    id_: str


@strawberry.type
class User:
    @strawberry.field
    @staticmethod
    async def name(parent: strawberry.Parent[UserRow]) -> str:
        return f"User Number {parent.id}"


@strawberry.type
class Query:
    @strawberry.field
    def user(self) -> User:
        return UserRow(id_="1234")

Here's the preview release card for twitter:

Here's the tweet text:

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

Get it here 👉 https://github.com/strawberry-graphql/strawberry/releases/tag/(next)

codecov · 2023-08-07T23:03:47Z

Codecov Report

Merging #3017 (baef780) into main (4eb629e) will decrease coverage by 0.05%.
Report is 18 commits behind head on main.
The diff coverage is 99.07%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3017      +/-   ##
==========================================
- Coverage   96.53%   96.49%   -0.05%     
==========================================
  Files         468      469       +1     
  Lines       29116    29255     +139     
  Branches     3582     3603      +21     
==========================================
+ Hits        28108    28230     +122     
- Misses        827      843      +16     
- Partials      181      182       +1

codspeed-hq · 2023-08-07T23:05:01Z

CodSpeed Performance Report

Merging #3017 will not alter performance

_{Comparing mattalbr:parent_without_error (baef780) with main (4eb629e)}

Summary

✅ 12 untouched benchmarks

mattalbr · 2023-08-10T19:30:56Z

FYI @patrick91 I think the code coverage error will be fixed in #3028

There's only one early exit line in real code that seems to be missing coverage

Going to try again without the ConflictingArgumentsError.

…round.

patrick91

This looks almost perfect :) I left a not about potentially throwing an error when using more than one of self/root/Parent

patrick91 · 2023-09-14T14:02:13Z

RELEASE.md

@@ -0,0 +1,3 @@
+Release type: minor
+
+Adds new strawberry.Parent type annotation to support resolvers without use of self.


can you add a quick example of how this works here? 😊

patrick91 · 2023-09-14T14:03:34Z

strawberry/parent.py

+...     def user(self) -> User:
+...         return UserRow(id_="1234")
+...
+"""


patrick91 · 2023-09-14T14:03:53Z

strawberry/private.py

-        )
-
-    return False
+    return type_has_annotation(type_, StrawberryPrivate)


patrick91 · 2023-09-14T14:04:37Z

strawberry/schema/schema_converter.py

+                if parent_parameter := field.base_resolver.parent_parameter:
+                    kwargs[parent_parameter.name] = source
+
+                if root_parameter := field.base_resolver.root_parameter:
                    kwargs[root_parameter.name] = source

-                info_parameter = field.base_resolver.info_parameter
-                if info_parameter:
+                if info_parameter := field.base_resolver.info_parameter:


patrick91 · 2023-09-14T14:06:10Z

tests/schema/test_resolvers.py

+# Should we allow these?
+def parent_and_self(self, parent: Parent[UserLiteral]) -> str:
+    return f"User {parent.id}"


I had the same question! I think it might confusing to allow both, so might be nice to throw a warning or error :)

Done! I went ahead and also enforced the same for self + root and some other combos under the generic principle of "identify each resource at most once"

…d types.

RELEASE.md

github-actions · 2023-09-14T22:34:55Z

Hi 👋 You can find a preview of the docs here:

https://strawberry.rocks/docs/pr/3017/errors/conflicting-arguments
https://strawberry.rocks/docs/pr/3017/types/resolvers

… non-self types. (strawberry-graphql#3017)

botberry added bot:has-release-file bot:release-type-minor labels Aug 10, 2023

mattalbr mentioned this pull request Aug 10, 2023

Exclude unit tests themselves from code coverage results. #3028

Closed

mattalbr added 5 commits August 10, 2023 14:28

First take at implementing strawberry.Parent.

fa5c66d

Going to try again without the ConflictingArgumentsError.

Remove ConflictingArgumentsError and add unit tests.

6567611

Fix lint and add release.md

bdda6ed

Actually add RELEASE.md

277d6ce

Simplify some tests to reduce missing code coverage, and move utils a…

a916ef9

…round.

mattalbr force-pushed the parent_without_error branch from aab8a82 to a916ef9 Compare August 10, 2023 21:34

Remove cached_property

768a272

patrick91 reviewed Sep 14, 2023

View reviewed changes

mattalbr added 4 commits September 14, 2023 20:51

Add a ConflictingArgumentsError that does more enforcement of reserve…

a80ef9d

…d types.

Remove obsolete comment.

91b147d

Add missing files.

34e7339

Fix lint errors.

b6bd527

patrick91 reviewed Sep 14, 2023

View reviewed changes

RELEASE.md Outdated Show resolved Hide resolved

mattalbr added 2 commits September 14, 2023 21:29

Fix windows rendering of errors via specifying encoding.

5640893

Update RELEASE.md per Patrick's comment.

51e8508

mattalbr requested a review from patrick91 September 14, 2023 21:38

Take a stab at documenting strawberry.Parent.

baef780

patrick91 added the ok-to-preview label Sep 14, 2023

patrick91 merged commit d1ff467 into strawberry-graphql:main Sep 14, 2023
57 of 58 checks passed

etripier pushed a commit to Greenbax/strawberry that referenced this pull request Oct 25, 2023

Add new strawberry.Parent annotation to support static resolvers with…

b6ef0ee

… non-self types. (strawberry-graphql#3017)

caspervk mentioned this pull request Nov 26, 2024

strawberry.Parent not supporting forward refs #3481

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new strawberry.Parent annotation to support static resolvers with non-self types. #3017

Add new strawberry.Parent annotation to support static resolvers with non-self types. #3017

mattalbr commented Aug 7, 2023

botberry commented Aug 7, 2023 •

edited

Loading

codecov bot commented Aug 7, 2023 •

edited

Loading

codspeed-hq bot commented Aug 7, 2023 •

edited

Loading

mattalbr commented Aug 10, 2023

patrick91 left a comment

patrick91 Sep 14, 2023

mattalbr Sep 14, 2023

patrick91 Sep 14, 2023

mattalbr Sep 14, 2023

patrick91 Sep 14, 2023

patrick91 Sep 14, 2023

patrick91 Sep 14, 2023

mattalbr Sep 14, 2023

github-actions bot commented Sep 14, 2023

		@@ -0,0 +1,3 @@
		Release type: minor

		Adds new strawberry.Parent type annotation to support resolvers without use of self.

Add new strawberry.Parent annotation to support static resolvers with non-self types. #3017

Add new strawberry.Parent annotation to support static resolvers with non-self types. #3017

Conversation

mattalbr commented Aug 7, 2023

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

botberry commented Aug 7, 2023 • edited Loading

codecov bot commented Aug 7, 2023 • edited Loading

Codecov Report

codspeed-hq bot commented Aug 7, 2023 • edited Loading

Merging #3017 will not alter performance

Summary

mattalbr commented Aug 10, 2023

patrick91 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 14, 2023

botberry commented Aug 7, 2023 •

edited

Loading

codecov bot commented Aug 7, 2023 •

edited

Loading

codspeed-hq bot commented Aug 7, 2023 •

edited

Loading