GraphQLBundle
GraphQLBundle copied to clipboard
Add support for multiple "argsBuilder"
| Q | A |
|---|---|
| Bug report? | no |
| Feature request? | yes |
| BC Break report? | no |
| RFC? | . |
| Version/Branch | . |
As discussed on a Slack thread, that would be awesome to have the possibility to write something like that:
myField:
argsBuilders: # argsBuilderS, not argsBuilder
-
builder: MyFirstArgsBuilder
config: ...
-
builder: MySecondArgsBuilder
config: ...
It would prevent to us to write something not flexible at all:
class MyFirstArgsBuilder implements MappingInterface
{
public function toMappingDefinition(array $config): array
{
$definition = [
'foo' => [
'type' => 'Int',
],
];
return $definition;
}
}
class MySecondArgsBuilder implements MappingInterface
{
public function toMappingDefinition(array $config): array
{
$definition = [
'bar' => [
'type' => 'Int',
],
];
return (new MyFirstArgsBuilder())->toMappingDefinition($config) + $definition;
}
}
Also, it should handle argument name conflicts. For example, it should throw an exception if MyFirstArgsBuilder and MySecondArgsBuilder both provide foo argument.
Thanks!
@Kocal What do you expect, when:
- ~~an args builder emits arg which already defined in configs~~
- an successive args builder emits arg which already emitted by previous arg(s) builders?
edit Think for the first its useful to behave like the argsBuilder currently does. The question is whether to propagate this logic to args builderS due to maybe confusing situation when you use multiple builders and they mess each other up. This is why with the ability to add types in builder(s) #473 I went the routine of denying overrides. The question is whether it gets confusing when all the builders behave differently in this case. So take this just as a thought for future and ignore the first point :)
What do you expect, when an successive args builder emits arg which already emitted by previous arg(s) builders?
Ah you're right, I didn't think about this. I didn't touch GraphQL/ArgsBuilders for months, but I have 2 propositions:
- Do a simply
array_mergewith definitions arrays - Modifiy the
toMappingDefinitionmethod to inject a new parameter that is the previous mapping definition, and let the user do what he wants:
class MySecondArgsBuilder implements MappingInterface
{
public function toMappingDefinition(array $config, array $parentDefinition): array
{
// Here, `$parentDefinition` is the result of the previous args builder `MyFirstArgsBuilder`
$definition = [
'bar' => [
'type' => 'Int',
],
];
return $parentDefinition + $definition;
}
}
I would not yet touch the MappingInterface as it is used in too many different places.
Maybe I phrase the focus of the question differently: Do you think an arg builder replacing args from other arg builder is rather not an error and you want to allow it or rather error you want to know about at compile time after changing the config.
... I don't know, both solutions seem good to me ¯\(ツ)/¯