strawberry icon indicating copy to clipboard operation
strawberry copied to clipboard

Rename directives parameter

Open patrick91 opened this issue 3 years ago • 2 comments
trafficstars

In #2054 we've added support for printing schema directives on the schema, while doing that we found some naming issues, currently we use a directives parameters on the following functions/classes:

  • strawberry.Schema
  • strawberry.field
  • strawberry.type
  • strawberry.input
  • strawberry.interface

the usage on strawberry.Schema is different than the one on all the other functions, on strawberry.Schema it refers to directives that are printed in the schema and can be used in from the clients, for example:

@strawberry.directive(
    locations=[DirectiveLocation.FIELD], description="Make string uppercase"
)
def uppercase(value: str, example: str):
    return value.upper()

schema = strawberry.Schema(query=Query, directives=[uppercase])

this returns a schema like this:

"""Make string uppercase"""
directive @uppercase(example: String!) on FIELD

type Query { ... }

and then a client can use them like so:

query {
    somefield @uppercase

in all the other cases directives are meant as schema directives, which are directives that are only meant to be printed in the schema (for use by other tools), for example the following code:

@strawberry.schema_directive(locations=[Location.SCHEMA])
class Tag:
    name: str

@strawberry.type
class Query:
    first_name: str = strawberry.field(directives=[Tag(name="team-1")])

schema = strawberry.Schema(query=Query, schema_directives=[Tag(name="team-1")])

will print the following schema:

directive @tag(name: String!) on SCHEMA

schema @tag(name: "team-1") {
    query: Query
}

type Query {
    firstName: String!
}

but tag is not meant to be used by clients.

So far we have only had support for schema directives on the fields and types, where you could only use schema directives so we named the parameter directives, like so:

@strawberry.type(directives=[Tag("sensitive")])
class AType:
     ...

unfortunately this didn't work for schema, since we use the directives flag for operation directives, like we have seen above.

For now I've added a new parameter called schema_directives, but this name is inconsistent with what we had already:

  • strawberry.Schema: directives here mean operation directives, so we use schema_directives for schema directives
  • strawberry.field: directives here mean schema directives
  • strawberry.type: directives here mean schema directives
  • strawberry.input: directives here mean schema directives
  • strawberry.interface: directives here mean schema directives

We should improve this, and we have two options:

1. always use directives

This is probably the easiest thing to do for our users, we can deprecate schema_directives on the schema and allow to pass all kinds of directives inside directives, then we can decide what to do based on the directive type, but I think this might be confusing to end users

2. rename directives to operation_directives and directives to schema_directives everywhere

This is more annoying than the option above as it requires more changes from our users but it is probably the option that makes more sense. It should make it easier to understand how directives are used in the schema 😊

Also I think not a lot of people are using custom directives in strawberry so hopefully we the number of people affected will be small. In any case we should do this with a deprecation step. We could write a codemod too, but that's probably overkill for this change :)

patrick91 avatar Aug 01 '22 10:08 patrick91

Well since the package didn't release V1.0, it is a brave decision to follow the option 2. I was also confused to understand the difference; until I realize that there are two types of directives.

I'd prefer to follow the same naming as in: https://www.apollographql.com/docs/apollo-server/schema/directives/#schema-directives-vs-operation-directives

schema_directives and operation_directives

ammar-faifi avatar Aug 01 '22 16:08 ammar-faifi

Well since the package didn't release V1.0, it is a brave decision to follow the option 2.

What do you mean? 😊 I think we make it easier to upgrade anyway, even if we are not in v1

I was also confused to understand the difference; until I realize that there are two types of directives.

Gotcha, it's good to know that this was confusing 😊

I'd prefer to follow the same naming as in: https://www.apollographql.com/docs/apollo-server/schema/directives/#schema-directives-vs-operation-directives

schema_directives and operation_directives

operation_directives sounds good to me, thanks for sharing this! I'll update the issue to use operation directives :)

patrick91 avatar Aug 02 '22 11:08 patrick91