strawberry icon indicating copy to clipboard operation
strawberry copied to clipboard

Fix _RESOLVER_TYPE for async functions

Open bricker opened this issue 2 years ago • 5 comments

Description

Fixes generic type arguments for Coroutine in RESOLVER_TYPE. The given types are incorrect: It is Coroutine[YieldType, SendType, ReturnType].

This allows catching when the return type of an async resolver is incompatible with the field type. For example:

async def resolver_func() -> int:
  ...

# currently not caught by typechecker
field: str = strawberry.field(resolver=resolver_func)

With the change made in this PR, the above code will be caught by typecheckers.

Types of Changes

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

Issues Fixed or Closed by This PR

  • Fixes #3071

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.
  • [ ] I have added tests to cover my changes.
  • [ ] I have tested the changes and verified that they work and don't break anything (as well as I can manage).

bricker avatar Nov 19 '23 02:11 bricker

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


This release improves type checking for async resolver functions when used as strawberry.field(resolver=resolver_func).

Now doing this will raise a type error:

import strawberry


def some_resolver() -> int:
    return 0


@strawberry.type
class User:
    # Note the field being typed as str instead of int
    name: str = strawberry.field(resolver=some_resolver)

Here's the tweet text:

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

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

botberry avatar Nov 19 '23 02:11 botberry

Codecov Report

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

Project coverage is 96.53%. Comparing base (014c4ec) to head (ffdc22d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3241   +/-   ##
=======================================
  Coverage   96.52%   96.53%           
=======================================
  Files         520      520           
  Lines       33243    33243           
  Branches     5527     5527           
=======================================
+ Hits        32089    32090    +1     
+ Misses        921      920    -1     
  Partials      233      233           

codecov[bot] avatar Nov 19 '23 02:11 codecov[bot]

CodSpeed Performance Report

Merging #3241 will not alter performance

Comparing bricker:bcr/2311/fix-coroutine-fields (ffdc22d) with main (014c4ec)

Summary

✅ 12 untouched benchmarks

codspeed-hq[bot] avatar Nov 19 '23 02:11 codspeed-hq[bot]

I am converting this to Draft while I investigate the test failures. I need help from the maintainers to understand these failures.

For the record: This change does fix the errors from Pyright (via Pylance in VSCode), but the test failures are from mypy.

bricker avatar Nov 19 '23 02:11 bricker

Unfortunately, according to Github, "the logs for this run have expired and are no longer available". This also hides the "re-run jobs" button for me. ~~Could you try to mark this PR as ready for review? Hopefully this will trigger CI again and I can take a look.~~ Edit: looks like marking the PR as ready for review didn't help. Need to figure out how to trigger CI again

DoctorJohn avatar Feb 28 '24 19:02 DoctorJohn

Dynamic field creation failed (or may be i doing something wrong).

Suppose you have code

def create_get_data_format_query(obj: SomeClass) -> tuple[str, Any, Field[DataSchema]]:
    async def resolver_func() -> DataSchema:
        db = session_context.get()

        s = await SomeClass.load_from_database(db, obj.id)

        if s is None:
            raise ValueError

        return await serialize_schema(s) # This function return DataSchema and typing is ok

    resolver: StrawberryField = strawberry.field(
        resolver=resolver_func,
        name="get_schema", description="Get schema",
    )

    return "get_schema", DataSchema, field(default=resolver)

And this produce following error

error: Argument "resolver" to "field" has incompatible type "Callable[[], Coroutine[Any, Any, DataSchema]]"; expected
"StrawberryResolver[Never] | Callable[..., Never] | Callable[..., Coroutine[Any, Any, Never]] | Callable[..., Awaitable[Never]] | staticmethod[Any, Never] | classmethod[Any, Any, Never]"  [arg-type]
            resolver=resolver_func,

Why DataSchema become Never? What do i do wrong or what do typing do wrong?

I tried to change StrawberryField to DataSchema - it not hepled.

Ansud avatar May 27 '24 10:05 Ansud

@Ansud it's not your fault, apparently this PR triggered a Mypy error, there's an initial fix here: https://github.com/strawberry-graphql/strawberry/pull/3516

patrick91 avatar May 27 '24 10:05 patrick91