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

Allow using instance of Field #521

Merged
merged 18 commits into from
Nov 12, 2019
Merged

Allow using instance of Field #521

merged 18 commits into from
Nov 12, 2019

Conversation

georgeboot
Copy link
Contributor

@georgeboot georgeboot commented Nov 11, 2019

This code enabled you to use an instance of a Field instead of only specifying it's class. This enables some cool reusability scenarios.

The problem we wanted to solve is reusability of both a field definition (with args) but also the resolver.

By default you define your field in a field class and your resolver (mapper) in a scalar.
But since the scalar can't access the $args, we opted to do it this way.

We're currently using it for our FormattableDate type.

<?php

class Purchase extends GraphQLType
{
    protected $attributes = [
        'name' => 'Purchase',
        'model' => PurchaseModel::class,
    ];

    public function fields(): array
    {
        return [
            // ...

            'createdAt' => new Fields\FormattableDate([
                'alias' => 'created_at',
            ]),
        ];
    }
}
<?php

class FormattableDate extends Field
{
    protected $attributes = [
        'name' => 'FormattableDate',
    ];

    public function __construct(array $settings = [])
    {
        $this->attributes = \array_merge($this->attributes, $settings);
    }

    protected function getProperty(): string
    {
        return $this->attributes['alias'] ?? $this->attributes['name'];
    }

    public function args(): array
    {
        return [
            'human' => [
                'type' => Type::boolean(),
                'defaultValue' => false,
            ],
            'format' => [
                'type' => Type::string(),
                'defaultValue' => 'Y-m-d H:i',
                'description' => 'Defaults to Y-m-d H:i',
            ],
            'relative' => [
                'type' => Type::boolean(),
                'defaultValue' => false,
            ],
        ];
    }

    public function type(): Type
    {
        return GraphQL::type('FormattableDate');
    }

    public function resolve(Model $root, array $args): string
    {
        /** @var Carbon $date */
        $date = $root->{$this->getProperty()};

        if ($args['human']) {
            return $date->format('l j F Y');
        }

        if ($args['relative']) {
            return $date->diffForHumans();
        }
        
        return $date->format($args['format']);
    }
}

Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach looks good to me!

Could you please:

  • add test for it?
    Since this requires custom types, they're usually put into tests/Unit/<NameOfTestUnit/ with their types, queries, etc.
  • add a your example to the documentation?

thanks!

src/Support/Type.php Outdated Show resolved Hide resolved
@georgeboot georgeboot requested a review from mfn November 12, 2019 10:14
Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid work, thank you 🤗

@mfn mfn merged commit a405ad8 into rebing:master Nov 12, 2019
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.

None yet

2 participants