lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

Enums collisions are not detected when validating schema with native enums

Open crezra opened this issue 4 years ago • 3 comments

Describe the bug When using native enums, as showcased here https://lighthouse-php.com/master/the-basics/types.html#native-php-definition coupled with https://github.com/BenSampo/laravel-enum Enum name collisions are not detected when running php artisan lighthouse:validate-schema

Expected behavior/Solution Validating schema should detect enum name collisions when using native php enums

Steps to reproduce

  1. install https://github.com/BenSampo/laravel-enum
  2. create a new enum MyEnum.php
<?php

declare(strict_types=1);

namespace App\Enums;

use BenSampo\Enum\Enum;

class MyEnum extends Enum
{
    public const DONE = 'DONE';
}   
  1. create a new graphql enum with the same name
enum MyEnum {
    DONE
}
  1. run php artisan lighthouse:validate-schema
  2. Output is The defined schema is valid. Output/Logs
Click to expand
# Add in log output/error messages here

Lighthouse Version 4.18.0

crezra avatar Oct 19 '21 17:10 crezra

Do you actually register your enum class with the TypeRegistry?

spawnia avatar Oct 20 '21 09:10 spawnia

yes

$typeRegistry->register(new LaravelEnumType(\App\Enums\MyEnums::class));

The php enum is working when I do not have the graphql enum defined

crezra avatar Oct 20 '21 09:10 crezra

This is tricky to validate, given we want generally want to leverage lazy loading. When calling validate-schema, the following happens in order:

  1. Laravel boots and calls service providers
  2. Presumably, a service provider registers the programmatic types within TypeRegistry
  3. The command is actually called
  4. The schema files are loaded and parsed
  5. Schema validation starts, traversing the schema starting from the root query types
  6. A usage of MyEnum is encountered, triggering a call to the lazy type loader - which is TypeRegistry::get()
  7. Since the programmatic type is already registered, the AST is never considered, thus no duplicate is detected

Lazy type loading is great for performance, we generally want to avoid loading all the types into the schema. During validation, we actually want that, but the order of registering the types and the mechanism of type loading makes it so that programmatic types have precedence and duplicates are undetected. I am sure we can fix this somehow, but we have to keep performance and convenience in mind.

spawnia avatar Oct 20 '21 14:10 spawnia