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

Changing signature of extended input type field still casts to the old signature. #454

Open
golaod opened this issue Apr 4, 2022 · 1 comment

Comments

@golaod
Copy link

golaod commented Apr 4, 2022

See below for a context and later I will describe result and expectation.

class FiltersFactory
{
    /**
     * @Factory
     */
    public function createFilters(?string $param = null): Filters
    {
        $filters = new Filters();
        if ($param) {
            $filters->add('param', $param);
        }
        return $filters;
    }
}

and now extended input

class FiltersDecorator
{
    /**
     * @Decorate(inputTypeName="FiltersInput")
     */
    public function addExtraFilter(Filters $previous, ?int $param = null)
    {
        if ($param) {
            $previous->add('param', $param);
        }

        return $previous;
    }
}

Query

product(filters:{param:"hello"})

Expectation without FiltersDecorator

Works well because param is of type string, so it's gonna be set through Filters factory

Result

The same

Expectation with FiltersDecorator

GraphQL throws an exception because param must by of type Int!

Result

The same


Fixed query

product(filters:{param:5})

Expectation

Works well because now type is int and param will be set through FiltersDecorat

Result

Internals throw exception String cannot represent a non string value: 5


Reporting this as a bug, but maybe overridding should not be possible in the first place, otherwise casting to old type should not happen. If I changed to int and sent int, then it's not string anymore.

@oojacoboo
Copy link
Collaborator

@golaod I'm not entirely sure what you're looking to accomplish at the end of the day, but I'm doubtful that the Decorate annotation is best solution. Nonetheless, I'm not sure, at least according to the docs, that it supports overriding an existing field. I've never used the Decorate annotation, and would do my best to keep it that way.

Why not use another field here if it's of a different type? Additionally, it's highly recommended in GraphQL to create more input types for different scenarios, as opposed to trying to do more with a single input type.

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

No branches or pull requests

2 participants