thecodingmachine / graphqlite

Use PHP Attributes/Annotations to declare your GraphQL API
https://graphqlite.thecodingmachine.io
MIT License
557 stars 98 forks source link

Make type namespaces available in RootTypeMapperFactoryContext #605

Closed mdoelker closed 1 year ago

mdoelker commented 1 year ago

Sometimes you need access to the type namespaces to resolve a type name to a class. The enum root type mappers that ship with Graphqlite work like this. Custom root type mappers do not have access to this information currently though, because it is missing in the factory context.

Built-in enum type mappers receive the list of namespaces:

if (interface_exists(UnitEnum::class)) {
    $rootTypeMapper = new EnumTypeMapper($rootTypeMapper, $annotationReader, $symfonyCache, $nsList);
}

if (class_exists(Enum::class)) {
    $rootTypeMapper = new MyCLabsEnumTypeMapper($rootTypeMapper, $annotationReader, $symfonyCache, $nsList);
}

RootTypeMapperFactoryContext is missing this info though:

$rootSchemaFactoryContext = new RootTypeMapperFactoryContext(
    $annotationReader,
    $typeResolver,
    $namingStrategy,
    $typeRegistry,
    $recursiveTypeMapper,
    $this->container,
    $namespacedCache,
    $this->globTTL,
);

I have a custom root type mapper for another enum library and am forced to pass in the type namespaces again:

->addTypeNamespace('My\\Custom\\Ns\\')
->addRootTypeMapperFactory(new MarcMabeEnumTypeMapperFactory(['My\\Custom\\Ns\\']))

This PR adds the list of namespaces to the factory context so that root type mapper factories can pass it on to the created mapper instance.

mdoelker commented 1 year ago

I realize that MyCLabsEnumTypeMapper is deprecated in favor of native enums, but would you be interested in a PR for MarcMabe/Enum support? There might be a couple of codebases that still rely heavily on it like ours where it is not feasible to migrate everything to native enums immediately. It also supports comments on enum values which I will look into providing for the other two enum implementations as well.

oojacoboo commented 1 year ago

@mdoelker I'd like to not add more BC stuff to this lib. We need to limit complexity as much as possible for the sake of maintenance. If you can work with an adapter for this, that's going to be best. We need to be moving forward. Migrations to native Enums really shouldn't be difficult.

mdoelker commented 1 year ago

@oojacoboo Sure, that's fair. Getting this PR's change in would then make that even easier and allow for a simple plugin-like usage, so it could be published as its own project.

oojacoboo commented 1 year ago

Thanks for the PR @mdoelker!