Skip to content
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

Support interfaces #35

Closed
simPod opened this issue Nov 22, 2021 · 6 comments · Fixed by #36
Closed

Support interfaces #35

simPod opened this issue Nov 22, 2021 · 6 comments · Fixed by #36
Labels
enhancement A feature or improvement

Comments

@simPod
Copy link
Collaborator

simPod commented Nov 22, 2021

I'm trying to generate classes for InterfaceType and it seems not to be supported in ClassGenerator. Is it intentional?

selection is like

interfacetype {
    somefield
}

but I'm crashing on

throw new \Exception('Unsupported type '.get_class($namedType).' found.');

Unsupported type GraphQL\Type\Definition\InterfaceType found.

Thanks

@spawnia spawnia changed the title InterfaceType support Support interfaces Nov 22, 2021
@spawnia spawnia added the enhancement A feature or improvement label Nov 22, 2021
@spawnia
Copy link
Owner

spawnia commented Nov 22, 2021

Yeah, interfaces are not supported yet. I will probably add support as soon as I have a use case for them. If you want to contribute, I am open to reviewing and aiding in the implementation of this feature.

@simPod
Copy link
Collaborator Author

simPod commented Nov 22, 2021

Hmm. So what if we add methods to generated classes that represent Object types and let them implement interfaces representing Interface types for shared fields?

@spawnia
Copy link
Owner

spawnia commented Nov 23, 2021

The generated result objects allow access to fields in the form of properties. I think this is more elegant than methods because:

  • it allows null-safe access with ?? (this will be less imporant with PHP 8 null safe method calls ?->)
  • no need to type an extra set of parantheses
  • easy setup of mocked results for test by just setting the properties

That said, interfaces without methods still provide value:

  • while they cannot define properties, they can indeed define virtual properties via @property. We will have to check how this interacts when the implementing object type narrows a field type, e.g. changing it to be non-nullable
  • it can be used as a strong type hint

@spawnia spawnia mentioned this issue Nov 23, 2021
3 tasks
@simPod
Copy link
Collaborator Author

simPod commented Nov 23, 2021

I see the PR, just few notes on those interfaces.

easy setup of mocked results for test by just setting the properties

methods would read properties so the same applies, no?

it allows null-safe access with ??

method calls do so? or I don't understand


Implemented interfaces only with properties seem a bit dirty as it is not how the language was designed

@spawnia
Copy link
Owner

spawnia commented Nov 23, 2021

Given this code:

class Foo {
    public ?Foo $bar;
    function bar(): ?Foo { return null; }
}

$foo = new Foo;

The following will work without error:

$foo->bar->bar ?? null;

This will throw with Fatal error: Uncaught Error: Call to undefined function bar():

$foo->bar()->bar() ?? null;

PHP 8 would allow to do it safely and succintly, but even then properties are more terse:

$foo->bar()?->bar();

@simPod
Copy link
Collaborator Author

simPod commented Nov 23, 2021

Ah ok I see what you meant by that. I go with PHP 8 everywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature or improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants