GraphQLite is automatically making interface implementations when it shouldn't

[MattBred]

Version: 5.0.1

According to https://graphqlite.thecodingmachine.io/docs/inheritance-interfaces, GraphQLite should only make an Impl type when it can't find another type that implements that interface:

If GraphQLite cannot find a proper GraphQL Object type implementing an interface, it will create an object type "on the fly".

However, it does it no matter what on my end:

/**
 * @GraphQL\Type
 */
interface MobileAppInterface
{
	/**
	 * @GraphQL\Field 
	 */
	public function getVersionCode(): ?int;
}

/**
 * @GraphQL\Type
 */
class AndroidApp implements MobileAppInterface
{
	public function getVersionCode(): ?int
	{
		// TODO: Implement getVersionCode() method.
	}
}

This generates the following schema:

interface MobileAppInterface {
  versionCode: Int
}

type MobileAppImpl implements MobileAppInterface {
  versionCode: Int
}

type AndroidApp implements MobileAppInterface {
  versionCode: Int
}

[oojacoboo]

@MattBred I've noticed the same thing. It's a bit harmless, as the interface isn't used, just generated in the schema. But, it is sloppy and confusing.

A PR on this would be awesome :)

[Lappihuan]

As you can see here: https://github.com/thecodingmachine/graphqlite/blob/3b97f9d020297658e32833e9b3d49e7b27c650dd/src/Mappers/RecursiveTypeMapper.php#L212-L223 Your type somehow is not yet part of the typeRegistry, might be a ordering issue in the recursive type mapper?

[oojacoboo]

It's probably related to the mapping cache in the RecursiveTypeMapper. Note that the Interface type names are different. The implementation around this is shaky. It mostly works, but an improved design is really needed.

[Lappihuan]

You are right, this seems quite naive. The interface type name is generated in this function called on line 214 above: https://github.com/thecodingmachine/graphqlite/blob/3b97f9d020297658e32833e9b3d49e7b27c650dd/src/NamingStrategy.php#L31-L34 It should actually check if there is a class in php implementing this interface. At least thats what the docs suggest it does but apparently not.

[oojacoboo]

There are two mappings as well, the actual FQCN and the GQL name. I don't find that these two mappings work well with each other either. IMO, we'd benefit from having a single mapping cache that's more robust.

[Lappihuan]

@oojacoboo what do you mean by two mappings?

[oojacoboo]

The RecursiveTypeMapper has:

• mappedClasses
• classToTypeCache
• classToInputTypeCache
• interfaceToClassNameMap

I found it frustrating to work with all of these different maps and think this responsibility would probably be better left to a mapping abstraction.

[aszenz]

I have ran into the same issue, this causes issues when generating types from schema, i have to handle the interface type even though i already know it would be one of N types

[oojacoboo]

A PR on this is certainly welcomed!