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

Extended types does not respect root @group(namespace) directive #402

Closed
robsontenorio opened this issue Oct 19, 2018 · 6 comments · Fixed by #406
Closed

Extended types does not respect root @group(namespace) directive #402

robsontenorio opened this issue Oct 19, 2018 · 6 comments · Fixed by #406
Labels
question Request for support or clarification

Comments

@robsontenorio
Copy link
Contributor

robsontenorio commented Oct 19, 2018

Describe the bug

Even setting a @group(namespace) directive on root types it will not reflect on extend types.

Expected behavior

The extend types should respect the root @group(namespace) directive. Is this an expected behavior? Should i repeat "the full namespace" in every single extended type?

Schema

type Mutation @group(namespace: "App\\Http\\GraphQL\\Mutations"){
  foo: String
}

# this DOES NOT WORK

extend type Mutation {
  doSomething(id: ID!): String
    @field(resolver: "SomeMutation@doSome")
}

# this WORKS

extend type Mutation @group(namespace: "App\\Http\\GraphQL\\Mutations"){
  doSomething(id: ID!): String
    @field(resolver: "SomeMutation@doSome")
}

# this WORKS

extend type Mutation {
  doSomething(id: ID!): String
    @field(resolver: "App\\Http\\GraphQL\\Mutations\\SomeMutation@doSome")
}

Output/Logs

  "error": {
    "message": "No class 'SomeMutation' was found for directive 'field'",
    "exception": "Nuwave\\Lighthouse\\Exceptions\\DirectiveException",
    "file": "/var/www/api/vendor/nuwave/lighthouse/src/Schema/Directives/BaseDirective.php",
    "line": 174,
    ....

Environment

Lighthouse Version: dev-master (a9c4c47)
Laravel Version: 5.7
PHP Version: 7.2

Additional context

Default namespaces does not work also for extend type Query like @paginate(builder: "Class@method")

type Query @group(namespace: "App\\Http\\GraphQL\\Queries") {
  me: User 
}

# DOES NOT WORK

extend type Query{
  searchSomething(nome: String!): [Something]     
    @paginate(builder: "SomethingQuery@search")
}

# WORKS

extend type Query{
  searchSomething(nome: String!): [Something]     
    @paginate(builder: "App\\Http\\GraphQL\\Queries\\SomethingQuery@search")
}
@spawnia
Copy link
Collaborator

spawnia commented Oct 19, 2018

That is indeed the expected behaviour, @group applies only to fields that are actually in the exact type they are defined on, and it only applies to @field and @complexity directives. I clarified the description in the docs: https://lighthouse-php.netlify.com/docs/directives.html#group

We might expand the directives the namespace applies to, but we should do it selectively. It does not make sense for every directive that mentions a class in any way. Feel free to open up a discussion in another issue if that is something you care about.

The following should work, but is currently broken. I just finished a fix for it.

extend type Mutation {
  doSomething(id: ID!): String
    @field(resolver: "SomeMutation@doSome")
}

@robsontenorio
Copy link
Contributor Author

robsontenorio commented Oct 20, 2018

@spawnia I'm looking forward #406 before opening a new discussion.

Are you considering default namespace resolution through @group(namespace) and/or lighthouse.php settings ?

If it resolves default namespaces from lighthouse.php, will close #394 👍

@spawnia
Copy link
Collaborator

spawnia commented Oct 20, 2018

The default namespaces queries/mutations from lighthouse.php apply to all fields that are defined on the root types. @group namespaces can be applied to any types and overwrite those from the config.

@robsontenorio
Copy link
Contributor Author

So, extended types will inherit default namespace resolution?

@spawnia
Copy link
Collaborator

spawnia commented Oct 20, 2018

If they are extensions of Query or Mutation the default namespace from the config applies.

@group will only ever influence direct children.

@robsontenorio
Copy link
Contributor Author

Nice! Will close #402 and #394

@spawnia spawnia added the question Request for support or clarification label Oct 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Request for support or clarification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants