Lighthouse: Method directive without resolver arguments

[olivernybroe]

I often have methods on my models which I want to use in graphql, like this.

    /**
     * Checks if the job is paused.
     *
     * @param bool $isPaused
     * @return bool
     */
    public function isPaused(bool $isPaused = true): bool
    {
        return ($this->is_paused ?? false) === $isPaused;
    }

With an argument on a type like this

paused: Boolean! @method(name: "isPaused")

Looking at it, there shouldn't really be a reason for it to fail, as it is not obvious that data is passed to my method.

However having the option to get the resolver arguments are still really useful and should still be there.
My solution for this is to add a new argument to the method directive called withResolver which is an boolean.

paused: Boolean! @method(name: "isPaused" withResolver: true)

And this argument defaults to false, so if it is not set, then the arguments won't be supplied to the method.

This will be a breaking change as it would require all users who uses the arguments to actually update their schema.

[olivernybroe]

The implementation is really easy, just don't wanna do it if a better solution is there or if you guys don't agree.

[spawnia]

We can easily switch the logic around to be a non-breaking change for now.

[olivernybroe]

Yes, however looking at the naming of the directive, I think it makes more sense that it works my proposed way, this way we there are no hidden information.

I think adding the non-breaking edition for now, will just give us another parameter to keep supporting even after we make the breaking change that inverts that logic.

I think it makes more sense to just do it the right way from the start instead.

Another solution could also be a new directive, for example right now I have fixed this locally by using a new directive called getter.

paused: Boolean! @getter(name: "isPaused")

[JelmerDV]

@olivernybroe for your use case you could just use Laravel their model accessors and no need for any directive as well.

Example:

type Model {
  id: Int!
  name: String!
}

And on your model you'd have a method:

public function getNameAttribute(): string
{
  return 'foo';
}

So no need to use @method at all in your use case, I think @method should be for passing arguments to the given method. Although right now it's ugly since you need to expect your model (you're already in) as a first argument, I think this is only useful if you could class@method the given method, but that's also not the case.

So my opinion would be:

1. or remove passing the first attribute

2. or instead allow the class@method syntax.

Personally I'd go for the second option because we like to keep our entities as clean as possible.

[spawnia]

@JelmerDV can you clarify the second option with the class@method syntax, maybe provide an example?

[JelmerDV]

@spawnia Right now the @method directive will call the method on the model its class, but it will also pass the model. So you will have redundant data, since you're in the model already. Right now it's like this:

type Model {
  id: Int!
  name(some_argument: Int): String! @method(name: "foo")
}

class Model {
  public function foo(Model $model, array $arguments)
  {
    // do something with arguments
    return 'bar';
  }
} 

So right now it will call the foo method in Model and also pass the model as a first argument. This passing of the model would be useful if we were possible to have the method in a different class than the model it self. But that isn't the case, because we can't use the class@method syntax. So that's why u can just remove passing of the model, so first argument of the method will become the arguments supplied from user input. Or just allow us to have the method in a different class, then it would be useful to have the model being passed as first argument.

[spawnia]

Right, that describes how @method could pass the arguments differently.

How would that class@method syntax look like?

[JelmerDV]

@spawnia the same as in other directives for example within the @builder you can use class@method syntax, but right now that's not possible for @method. If this would be possible we could have these methods in a different class than the model it self, since the model is being passed as first argument.

[spawnia]

I see. You can just use @field for that.

[JelmerDV]

Ahh! My bad, didn't know about that directive. Nice to see it's already included in Lighthouse! Thank you. Although then my opinion would be the first argument, remove the passing of the model to the model its method, because it's redundant.

[olivernybroe]

Okay, so just to come with another example and with a new proposed structure.

Let's say I have this method in a class called Skill

    public function getName(string $locale = null): string
    {
        return SkillTranslator::from($locale)->($this->name);
    }

It will return the name of skill in the specified locale.

Using the method directive I would then write

type Skill {
    name(locale: String): String! @method(name: "getName", args: ["locale"])
}

This will result in the first argument supplied to the method will be the parameter locale.

This same structure will also make my first example available by then defining it as

paused: Boolean! @method(name: "isPaused" args: [])

If the parameter args is not supplied then it will supply the same data as it currently does.

[spawnia]

@olivernybroe interesting!

I like that this allows controlling the order in which arguments are passed. How about naming it pass instead of args?

[olivernybroe]

@spawnia Sure that could also work, think I thought of using args because we use that name in can directive also.

[spawnia]

That is specifically why i want to name it differently, the semantics are different:
@can(args: passes statically defined literal values. injectArgs is a bit closer, but only allows boolean configuration values.

[olivernybroe]

@spawnia Alright perfect, let's go with pass to differentiate them better.

[JasonTheAdams]

Oh wow! I was just thinking today how helpful this would be with the @method directive and here we are! Just to chime in, being able to pass in the field args and specify the parameter order and corresponding argument sounds perfect.

[olivernybroe]

@JasonTheAdams Nice to hear that you are a fan of this.
I unfortunately don't have time to work on this atm. so if anyone else want to implement it, please go ahead 😃

[JasonTheAdams]

I'm going to take a stab at this tomorrow. To be clear, this is the final syntax we're looking for, right?

type Skill {
    name(locale: String): String! @method(name: "getName", pass: ["locale"])
}

[olivernybroe]

@JasonTheAdams Sounds awesome!

Yep, that is the right syntax, don't hesitate to write here or slack to us if you have any issues.

[faiverson]

I have a question:
I read in this thread that @method it is a kind of getter function. My question is:
can be used as a setter function too? Or there is a better directive for that?
For example:

use case A:

input createUser {
    active: String! @method(name: "active")
}

public function active(): string
{
      return is_null($this->active) || $this->active === 'true';
}

use case B

input inputUser {
  username: String!
  first_name: String!
  last_name: String!
  email: String!
  password: String!
}

type User {
  username: String!
  name: String!
  email: String!
  password: String!
}

createUser(input: inputUser! @spread): User
      @method(name: "userCreate")
      @create

public function userCreate(array $args): string
    {
            if(empty($args['first_name'] ) || empty($args['last_name'])) 
               $args['name'] = 'John Doe';
           else 
              $args['name'] = $args['first_name'] . " " . $args['last_name'];
    }

Is this the right directive for that?

[spawnia]

Instead of passing down the usual resolver arguments, the @method directive will
now pass just the arguments given to a field. This behaviour could previously be
enabled through the passOrdered option, which is now removed.

type User {
    purchasedItemsCount(
        year: Int!
        includeReturns: Boolean
    ): Int @method
}

The method will have to change like this:

-public function purchasedItemsCount($root, array $args)
+public function purchasedItemsCount(int $year, ?bool $includeReturns)

[spawnia]

@faiverson there is currently nothing like that implemented.