-
Notifications
You must be signed in to change notification settings - Fork 99
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
Improvements on custom scalar types #178
Comments
I was finally able to get custom scalar types working. The issue, at the end of the day was that the mapper must not return a new instance of the GraphQL type. By using a static cached property of the type object on the mapper, it resolves this issue. For those that might come across this confusing error and see this issue, the error in question is:
The error message made me think that the getter method that was annotated had to return the actual GraphQL type and that I couldn't map it accordingly. It would still be really helpful for the docs to explain more on how to write and register custom scalar types. I think this is a very common need for people and I agree with the reference implementation's opinion of not implementing these by default as there is a wide variety of needs for each application. Therefore, I suggest the removal of the |
I'm reopening this issue. I've updated to the latest code and things have broken with type mapping. I have most of it pieced together again, but the custom scalar type mapping is broken again. The docs on this subject aren't exhaustive enough. I have no idea how to handle the following: /**
* @GraphQLite\Type()
*/
class Product
{
protected Money $amount;
/**
* @GraphQLite\Field()
*/
public function getAmount(): Money
{
return $this->amount;
}
} How do I tell GraphQLite how to handle a Does this |
Okay, after some digging, I was able to figure out the issue. I'll leave some details here for other folks that might come across this. Firstly, you're going to want to create a new scalar type object that extends Here is my custom type mapper that I created which handles all of our custom types: <?php
declare(strict_types = 1);
namespace Acme\GraphQL\Type;
use GraphQL\Type\Definition\InputType;
use GraphQL\Type\Definition\NamedType;
use GraphQL\Type\Definition\OutputType;
use GraphQL\Type\Definition\ScalarType;
use phpDocumentor\Reflection\DocBlock;
use phpDocumentor\Reflection\Type;
use Acme\Cog\Date;
use Acme\GraphQL\Type\Scalar\Date as DateType;
use TheCodingMachine\GraphQLite\Mappers\Root\RootTypeMapperInterface;
/**
* Handles the type mapping for GraphQL
*
* @author Jacob Thomason <[email protected]>
*/
class Mapper implements RootTypeMapperInterface
{
protected RootTypeMapperInterface $next;
protected array $typeMaps;
/**
* Constructor
*
* @param RootTypeMapperInterface $next
*/
public function __construct(RootTypeMapperInterface $next)
{
$this->next = $next;
// Define your type maps here
$this->typeMaps = [
'Date' => [Date::class => new DateType()],
];
}
/**
* Gets the type maps
*/
protected function getTypeMaps(): array
{
return $this->typeMaps;
}
/**
* Gets the respective type for a type name
*
* @param Type $type
*/
protected function getType(Type $type): ?ScalarType
{
$fqcn = ltrim((string)$type, '\\');
foreach ($this->getTypeMaps() as $map) {
if (isset($map[$fqcn])) {
return $map[$fqcn];
}
}
return null;
}
/**
* Returns a GraphQL type by name.
* If this root type mapper can return this type in "toGraphQLOutputType" or "toGraphQLInputType", it should
* also map these types by name in the "mapNameToType" method.
*
* @param string $typeName The name of the GraphQL type
*/
public function mapNameToType(string $typeName): NamedType
{
if (isset($this->getTypeMaps()[$typeName])) {
return array_values($this->getTypeMaps()[$typeName])[0];
}
return $this->next->mapNameToType($typeName);
}
/**
* Handles the conversion to an input type
*
* @param Type $type
* @param InputType|null $subType
* @param string $argumentName
* @param \ReflectionMethod $refMethod
* @param DocBlock $docBlockObj
*/
public function toGraphQLInputType(
Type $type,
?InputType $subType,
string $argumentName,
\ReflectionMethod $refMethod,
DocBlock $docBlockObj
): InputType
{
return $this->getType($type)
?: $this->next->toGraphQLInputType($type, $subType, $argumentName, $refMethod, $docBlockObj);
}
/**
* Handles the conversion to an output type
*
* @param Type $type
* @param OutputType|null $subType
* @param \ReflectionMethod $refMethod
* @param DocBlock $docBlockObj
*/
public function toGraphQLOutputType(
Type $type,
?OutputType $subType,
\ReflectionMethod $refMethod,
DocBlock $docBlockObj
): OutputType
{
return $this->getType($type)
?: $this->next->toGraphQLOutputType($type, $subType, $refMethod, $docBlockObj);
}
} Now, the only thing you need to do is define your "type maps" in the constructor. The key in the array is the type name you're using in your GraphQLite annotations. The array value assigned to that key is your domain object/type mapped to the scalar type that Webonyx graphql lib expects. Honestly, I think you could add an interface to your domain objects/types, and have one class that implements the |
Hey @oojacoboo , You are certainly right. And defining a scalar type is certainly something common enough to try to simplify it. Your Out of curiosity, if you were to define an interface for Scalar types, how would you design it? |
@moufmouf honestly, after a little more thought here, I think I'd prefer to not pollute my domain objects/scalars with the particulars of GraphQL. The class that I've written above is fairly agnostic. The only thing that's required is the mapping in the constructor. If that mapping could be exposed and state managed through the config, that'd be ideal, IMO. You'd just provide the simple Of course, the highly customizable nature of the I haven't looked into the internals here, but there are some inconsistencies with the strings being returned and used throughout the At the end of the day, having a simple |
Thanks @oojacoboo you've been a much of help. I was able to extract a CustomScalar class that's inherited by Email, Password, Money and other ValueObject types I need to map. I was able to register type mapper factory (in Laravel) like this:
I agree this should be part of the core, to be configured by the user in the end and also done a little bit more automatically. Consider this example:
So:
Would automatically return:
Also I was looking for some standard describing ValueObject interfaces but found none.
But found no standard for it. |
@mailopl glad that was helpful. While you're right that those classes could be handled automatically, I personally am not a fan of that approach and much prefer explicit definitions. I'm not a fan of magic methods, in general, including Regardless, I think there is a much more simplified way of handling this for most all scalar cases and the need to add a custom type mapper/factory is unnecessary. |
I've been struggling with getting a custom scalar type working with GraphQLite. I'm running into a number of issues, but at the end of the day, I'm just not clear on how everything is supposed to work.
The type mappers are quite confusing as there seem to be a lot of them used by GraphQLite internally. IMO, the library shouldn't use anything but the basic scalar types by default, and allow you to add custom, or pre-built, type mappers/scalar types during configuration.
Overall though, I just don't really understand how to get this working. A full code example would be awesome. In fact, an entire sample codebase that shows how everything is wired up would be even more helpful.
Most of the confusion stems from 2 places.
$factory->addTypeNamespace()
method? Do you just$factory->addRootTypeMapper()
and allow it to handle the mapping without needing to register the custom scalar?The text was updated successfully, but these errors were encountered: