Add directive `@inherits` for reuse of parent type definitions

[yondifon]

• [x] Added or updated tests
• [x] Documented user facing changes
• [x] Updated CHANGELOG.md

Changes Introduced a new Directive @extends that makes it easier to inherit from a parent type. This copies the merges the properties of the child to the parent while overriding existing properties.

[enzonotario]

Hi there! I've made a similar directive for a personal project, so let me introduce those changes and if you like it, we can continue working from here.

[spawnia]

In order for this merge request to move forward please address each of my comments.

[enzonotario]

Yes you're right. So, we have to define what kind of types it should handle (so far I think ObjectTypes, Inputs and Enums). So something like this should work:

        switch ($typeDefinition->kind) {
            case 'EnumTypeDefinition':
                $typeDefinition->values = $parentType->values->merge($typeDefinition->values);
                break;
            case 'ObjectTypeDefinition':
            case 'InputTypeDefinition':
                $typeDefinition->fields = $parentType->fields->merge($typeDefinition->fields);
                break;
        }

[yondifon]

Yes you're right. So, we have to define what kind of types it should handle (so far I think ObjectTypes, Inputs and Enums). So something like this should work:

        switch ($typeDefinition->kind) {
            case 'EnumTypeDefinition':
                $typeDefinition->values = $parentType->values->merge($typeDefinition->values);
                break;
            case 'ObjectTypeDefinition':
            case 'InputTypeDefinition':
                $typeDefinition->fields = $parentType->fields->merge($typeDefinition->fields);
                break;
        }

Actually, this solved the issue. Since it checks to see if the parent and child are of the same type.

@spawnia. Please can you review this, I don't think it will be an issue. There's a check that ensures that the two types in question are of the same so obviously have the same attributes.

1st Commits

[spawnia]

@yondifon I think your approach should in fact work, but it has some problems.

NodeList::merge() does not eliminate duplicates. Use ASTHelper::mergeUniqueNodeLists() instead. I would like to see a test case where such a duplicate is present and resolved correctly, with the child items having precedence.

Why is there special treatment for the name attribute?

[yondifon]

@yondifon I think your approach should in fact work, but it has some problems.

NodeList::merge() does not eliminate duplicates. Use ASTHelper::mergeUniqueNodeLists() instead. I would like to see a test case where such a duplicate is present and resolved correctly, with the child items having precedence.

Why is there special treatment for the name attribute?

😄 I don't know why I did that on the name attribute.

Thanks for the tip. I've restructured the tests to use buildschema. it's a lot easy and accurate.

But I think it's all good now. I think we've resolved all the comments. Here's the latest commit that takes care of it. (https://github.com/nuwave/lighthouse/pull/1895/commits/53b5cb9af0aa9158971e5690a475634c2ac4076c)

Might need some formatting and minor fixes but it's all good.

[spawnia]

Looks much better, definitely on the right track.

I think we've resolved all the comments.

There are still 3 open comments which are not sufficiently addressed.

[yondifon]

Looks much better, definitely on the right track.

I think we've resolved all the comments.

There are still 3 open comments which are not sufficiently addressed.

They are outdated

I think it was for documentation. Just check the documentation once more. @enzonotario tweaked it a lil.

[spawnia]

They are outdated

No. I have looked at them. They are still relevant. Please address them.

[yondifon]

They are outdated

No. I have looked at them. They are still relevant. Please address them.

Hey @spawnia. Just committed this (https://github.com/nuwave/lighthouse/pull/1895/commits/952a3706523b8af1068af9bec9e3655dd42f0037).

Check this out. But I think you get the whole idea behind the whole, you can lil tweaks to better document the PR.