Add metadata (the graphql.core `Node`) to `GraphQLField` and `GraphQLObject`. #3182

mgilson · 2023-10-30T18:46:19Z

Clients can use this metadata do specializilation during the creation of client types.

Description

GraphQL is a very flexible language where custom directives can be added for a large number of purposes. It's possible that in some cases, a custom directive on a query object or field could be used to impact what types are emitted on a client. e.g. a client might want to make fields nullable if they have a@skip or @include directive. Since directives can be as diverse as the people who make them, we should allow that information to be passed to the client generator plugin which can then be used or ignored as necessary.

Since it's unclear what these custom directives would do or how someone might use them in the general case, for now, I'm just passing the whole graphql node for the field or type through. In my concrete use-case, I have reasons to want to supply default values to the generated types from the client -- e.g.:

@dataclass
class Foo:
     field: str = "Hello world"

Rather than

@dataclass
class Foo:
     field: str

which is what gets generated by my plugin currently.

My graphql would look something like:

query MyQuery {
    getFoo {
        field @defaultValue(value: "Hello world")
    }
}

And this is fine as long as the server is taught to ignore the directive (or the directive is stripped before the query actually gets issued which is how my plugin actually works).

Types of Changes

Issues Fixed or Closed by This PR

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
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).

botberry · 2023-10-30T18:47:54Z

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

Augment the codegen GraphQLObjectType and GraphQLField with the graphql.language.ast.Node that caused the
respective object to be created. This node can be introspected for additional metadata for codegen plugins to use
for specialization of type creation.

Here's the tweet text:

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

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

codecov · 2023-10-30T18:48:51Z

Codecov Report

Merging #3182 (85bb731) into main (101ab96) will increase coverage by 0.00%.
The diff coverage is 97.72%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3182   +/-   ##
=======================================
  Coverage   96.40%   96.41%           
=======================================
  Files         498      498           
  Lines       31137    31220   +83     
  Branches     3815     3822    +7     
=======================================
+ Hits        30017    30100   +83     
+ Misses        914      912    -2     
- Partials      206      208    +2

codspeed-hq · 2023-10-30T18:54:40Z

CodSpeed Performance Report

Merging #3182 will not alter performance

_{Comparing mgilson:code-gen-field-metadata (85bb731) with main (101ab96)}

Summary

✅ 12 untouched benchmarks

patrick91 · 2023-10-30T22:25:15Z

strawberry/codegen/types.py

@@ -32,6 +34,7 @@ class GraphQLField:
    alias: Optional[str]
    type: GraphQLType
    default_value: Optional[GraphQLArgumentValue] = None
+    definition_node: Optional[Node] = None


do you think it would be possible to use the strawberry type here instead?

We could do both...

The query is the part that is controlled by the client (they may not control the upstream strawberry type). I actually have a branch that I haven't quite gotten working that tries to build a client from only a graphql schema (then the Python client could be generated against things that don't even use strawberry as the graphql provider).

But... It would be nice to have access to the strawberry type where available. That's probably easier to work with than digging through the graphql AST...

patrick91 · 2023-11-03T09:41:10Z

pyproject.toml

@@ -1,7 +1,7 @@
 [tool.poetry]
 name = "strawberry-graphql"
 packages = [ { include = "strawberry" } ]
-version = "0.211.1"
+version = "0.211.1.dev1"


Suggested change

version = "0.211.1.dev1"

version = "0.211.1"

Oops, that wasn't supposed to make it into the PR 😅

patrick91 · 2023-11-03T09:50:34Z

strawberry/codegen/query_codegen.py

+                    variable.name,
+                    None,
+                    variable_type,
+                    definition_node=variable_definition,


should this have strawberry_field as well?

Hmmm ... originally, I didn't think so ... but now that I think about it a little more, possibly ...

This represents the input variables to the operation. I guess this is represented by the query/mutation operation object?

Let me see what I can cook up.

just for more context, I'd like to keep as abstracted as possible 😊 we might change the internal representation (ie moving away from GraphQL) in future

So ... the more I dig into this, the more that this feels weird (or at least really complex). For example, you could have a query like:

query ($skipTitle: Boolean!, $val: Int!) { queryPost(val: $val) { id title @skip(if: $skipTitle) text } }

In this case, the $skipTitle variable field really has nothing to do with any of the types in the backend schema definition. The val field presumably does, but it's buried in the Query.query_post field's base_resolver (I guess you'd introspect the wrapped_func on that resolver to understand the backend metadata about this thing?)

For now, I think that I'll make the graphql_type on the {type}Variables point at the Query or Mutation class and then users can crawl that class however they want in their plugins. If we ever see more concrete use-cases, we can make better decisions on how we want to set these pieces of metadata to support those use-cases at that point.

Does that sound reasonable to you?

patrick91 · 2023-11-03T09:53:08Z

strawberry/codegen/types.py

+    definition_node: Optional[Node] = None
+    strawberry_type: Optional[StrawberryType] = None


I wonder about the naming here :) I think I'd make something like this:

Suggested change

definition_node: Optional[Node] = None

strawberry_type: Optional[StrawberryType] = None

graphql_node: Optional[Node] = None

graphql_type: Optional[StrawberryType] = None

what do you think?

Sure, I'm not picky about the naming here.

let's do the change then 😊

:face_palm: That's a little embarassing 😅

mgilson · 2023-11-03T19:35:15Z

It looks like pyright released a new version yesterday -- I'm guessing that's why all the pyright tests are now failing :).

womp womp.

patrick91

almost ready to go :)

patrick91 · 2024-01-09T14:57:17Z

strawberry/codegen/query_codegen.py

+            # (e.g. the ``Query`` or ``Mutation`` class and then we'll let consumers sort
+            # out how they want to use this metadata).
+            if variables_type.graphql_type is not None:
+                raise ValueError(


(same q for the one above) when could this happen?

if it is just a defensive check, let's add a nocover comment 😊

Yes, this is defensive to make sure we didn't mess something up.

These are the {Type}Variables objects that I mentioned in the other comments. Basically, at the client level, it's just a dataclass that describes the inputs to a strawberry resolver (rather than being an actual type on the strawberry/python side).

In that case, I made the decision to just set the graphql node to the Operation node and I set the graphql type to the query/mutation type.

Actually, I think that it's a little more complex ... It maps to the variables in the operation

query FooQuery($id: Int, $fetchBar: Boolean) { foo(id: $id) { bar @skip(if: $fetchBar) { x y } } }

The client is going to generate an object:

class FooQueryVariables: id: int fetch_bar: bool

This object doesn't map to any class/field on the strawberry/server side.

patrick91 · 2024-01-09T14:59:57Z

strawberry/codegen/query_codegen.py

+                    variable.name,
+                    None,
+                    variable_type,
+                    graphql_node=variable_definition,


can we use named arguments here?

and could we find where the variable is used and attach that field here? Might not probably worth, but maybe we can leave a comment about that? (The current comment is a bit unclear to me)

I've tried to update the comment to make it more clear. These are for the {Type}Variables objects which generally correspond to something like:

class Query: @strawberry.field def resolver_function(self, id: int, x: float) -> ReturnType: ...

The client is going to make a:

class QueryResolverFunctionVariables: id: int x: float

type of class (depending on the query generated). But in this case, there isn't a StrawberryField that maps 1-to-1 with the id parameter.

patrick91 · 2024-01-09T15:01:09Z

strawberry/codegen/types.py

+    definition_node: Optional[Node] = None
+    strawberry_type: Optional[StrawberryType] = None


let's do the change then 😊

patrick91 · 2024-01-09T15:02:53Z

tests/codegen/conftest.py

@@ -120,6 +130,14 @@ def get_person_or_animal(self) -> Union[Person, Animal]:
        p_or_a.age = 7
        return p_or_a

+    @strawberry.field
+    def get_person_with_inputs(self, name: str, age: int = 72) -> Person:


Suggested change

def get_person_with_inputs(self, name: str, age: int = 72) -> Person:

def get_person_with_inputs(self, name: str, age: int = 72) -> Person: # pragma: no cover

… this metadata do specialize the creation (e.g. adding a `@defaultValue` directive to add a default value to the generated objects).

mgilson · 2024-03-12T18:19:08Z

@patrick91 -- I believe that this is ready for another look when you have a few moments. Thanks!

mgilson force-pushed the code-gen-field-metadata branch from 3ca2fe1 to faa6754 Compare October 30, 2023 18:50

botberry added bot:has-release-file bot:release-type-minor labels Oct 30, 2023

mgilson force-pushed the code-gen-field-metadata branch 3 times, most recently from 35577c3 to 590d320 Compare October 30, 2023 21:13

strawberry-graphql deleted a comment from botberry Oct 30, 2023

patrick91 reviewed Oct 30, 2023

View reviewed changes

mgilson force-pushed the code-gen-field-metadata branch 5 times, most recently from f7631c6 to eb211c8 Compare October 31, 2023 14:29

patrick91 reviewed Nov 3, 2023

View reviewed changes

mgilson force-pushed the code-gen-field-metadata branch from eb211c8 to c0921ac Compare November 3, 2023 14:57

mgilson force-pushed the code-gen-field-metadata branch from c0921ac to 2295a61 Compare January 9, 2024 14:22

patrick91 reviewed Jan 9, 2024

View reviewed changes

mgilson force-pushed the code-gen-field-metadata branch 2 times, most recently from 7aab7e1 to 50dd99c Compare January 9, 2024 16:55

Add RELEASE.md file.

1f156a9

mgilson force-pushed the code-gen-field-metadata branch from 75a1c7d to 06d3afb Compare March 12, 2024 14:14

Add metadata (the graphql.core Node) to GraphQLField. Clients can use…

85bb731

… this metadata do specialize the creation (e.g. adding a `@defaultValue` directive to add a default value to the generated objects).

mgilson force-pushed the code-gen-field-metadata branch from 796bd90 to 85bb731 Compare March 12, 2024 14:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metadata (the graphql.core `Node`) to `GraphQLField` and `GraphQLObject`. #3182

Add metadata (the graphql.core `Node`) to `GraphQLField` and `GraphQLObject`. #3182

mgilson commented Oct 30, 2023

botberry commented Oct 30, 2023 •

edited

Loading

codecov bot commented Oct 30, 2023 •

edited

Loading

codspeed-hq bot commented Oct 30, 2023 •

edited

Loading

patrick91 Oct 30, 2023

mgilson Oct 30, 2023

patrick91 Nov 3, 2023

mgilson Nov 3, 2023

patrick91 Nov 3, 2023

mgilson Nov 3, 2023

patrick91 Nov 3, 2023

mgilson Nov 3, 2023

patrick91 Nov 3, 2023

mgilson Nov 3, 2023

patrick91 Jan 9, 2024

mgilson Jan 9, 2024 •

edited

Loading

mgilson commented Nov 3, 2023

patrick91 left a comment

patrick91 Jan 9, 2024

mgilson Jan 9, 2024

mgilson Jan 9, 2024

patrick91 Jan 9, 2024

mgilson Jan 9, 2024

patrick91 Jan 9, 2024

patrick91 Jan 9, 2024

mgilson commented Mar 12, 2024

		definition_node: Optional[Node] = None
		strawberry_type: Optional[StrawberryType] = None

	def get_person_with_inputs(self, name: str, age: int = 72) -> Person:
	def get_person_with_inputs(self, name: str, age: int = 72) -> Person: # pragma: no cover

Add metadata (the graphql.core Node) to GraphQLField and GraphQLObject. #3182

Are you sure you want to change the base?

Add metadata (the graphql.core Node) to GraphQLField and GraphQLObject. #3182

Conversation

mgilson commented Oct 30, 2023

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

botberry commented Oct 30, 2023 • edited Loading

codecov bot commented Oct 30, 2023 • edited Loading

Codecov Report

codspeed-hq bot commented Oct 30, 2023 • edited Loading

Merging #3182 will not alter performance

Summary

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgilson Jan 9, 2024 • edited Loading

Choose a reason for hiding this comment

mgilson commented Nov 3, 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

mgilson commented Mar 12, 2024

Add metadata (the graphql.core `Node`) to `GraphQLField` and `GraphQLObject`. #3182

Add metadata (the graphql.core `Node`) to `GraphQLField` and `GraphQLObject`. #3182

botberry commented Oct 30, 2023 •

edited

Loading

codecov bot commented Oct 30, 2023 •

edited

Loading

codspeed-hq bot commented Oct 30, 2023 •

edited

Loading

mgilson Jan 9, 2024 •

edited

Loading