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

First draft at returning strawberry types from mapped resolvers. #16

Closed
wants to merge 3 commits into from

Conversation

mattalbr
Copy link
Contributor

@mattalbr mattalbr commented Apr 27, 2023

Strawberry doesn't actually type check the values returned from
resolvers -- the "self" in resolvers is really just whatever object the
parent resolver returned, transparently forwarded by strawberry. Before
this change, all of our resolvers return (and in turn only support)
"self" being an instance of a sqlalchemy model, not the strawberry type.

At best, that's confusing, but at worst, it doesn't allow manually
defined resolvers on a mapped type to call sibiling resolvers. E.g.
imagine a user-defined resolver:

```python
@mapper.type(models.MyModel)
class MyModel:
  def resolve_other_field(self, info) -> int:
    return 5

  def resolve_some_field(self, info) -> int:
    return self.resolve_other_field(info) + 5
```

This fails, because the sqlalchemy model type has no
"resolve_other_field" defined, which is perplexing, because it seems
like it should be defined. The only way to call it is like:
MyModel.resolve_other_field(self, info), but IMO this unnecessarily
exposes the user to strawberry internal knowledge (static method
nonsense) that they don't need to be privvy to. By instead ensuring all
of our resolvers return the strawberry type, all the typing works just
like expected.

Strawberry doesn't actually type check the values returned from
resolvers -- the "self" in resolvers is really just whatever object the
parent resolver returned, transparently forwarded by strawberry. Before
this change, all of our resolvers return (and in turn only support)
"self" being an instance of a sqlalchemy model, not the strawberry type.

At best, that's confusing, but at worst, it doesn't allow manually
defined resolvers on a mapped type to call sibiling resolvers. E.g.
imagine a user-defined resolver:

```python
@mapper.type(models.MyModel)
class MyModel:
  def resolve_other_field(self, info) -> int:
    return 5

  def resolve_some_field(self, info) -> int:
    return self.resolve_other_field(info) + 5
```

This fails, because the sqlalchemy model type has no
"resolve_other_field" defined, which is perplexing, because it seems
like it should be defined. The only way to call it is like:
`MyModel.resolve_other_field(self, info)`, but IMO this unnecessarily
exposes the user to strawberry internal knowledge (static method
nonsense) that they don't need to be privvy to. By instead ensuring all
of our resolvers return the strawberry type, all the typing works just
like expected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant