Skip to content

Allow interfaces to implement other interfaces #728

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

Closed
Kingdutch opened this issue Sep 25, 2020 · 3 comments · Fixed by #740
Closed

Allow interfaces to implement other interfaces #728

Kingdutch opened this issue Sep 25, 2020 · 3 comments · Fixed by #740

Comments

@Kingdutch
Copy link
Contributor

Kingdutch commented Sep 25, 2020

v15 of graphql-js implemented an addition to the GraphQL spec which allows interfaces to implement other interfaces. It would be nice to be able to do this in PHP as well. For example when there is a generic Node interface and a specialised Media Node type needs to be created for images, videos, etc.

graphql-js implementation: graphql/graphql-js#2084
spec addition: graphql/graphql-spec#373

Article on why this is useful: https://dev.to/mikemarcacci/intermediate-interfaces-generic-utility-types-in-graphql-50e8

The following GraphQL schema snippet currently doesn't parse but would be expected to parse after implementation.

interface Node {
  uuid: ID!
}

interface Media implements Node {
  uuid: ID!
  url: String!
}

type Image implements Media & Node {
  uuid: ID!
  url: String!
}
@spawnia
Copy link
Collaborator

spawnia commented Sep 25, 2020

A PR for this would certainly be welcome.

I am on the fence about this counting as a breaking change. Using the feature in the schema definitely can break clients who do not know how to deal with this spec addition.

Depending on the implementation this might be non-breaking as in: users of this package can upgrade and all their code will still work.

@vladar
Copy link
Member

vladar commented Oct 6, 2020

This will be a part of the 15.0 release as it is indeed breaking. I am going to prepare a roadmap for 15.0 with a list of issues/tasks to guide anyone willing to contribute (no ETA though).

Kingdutch added a commit to Kingdutch/graphql-php that referenced this issue Nov 26, 2020
Kingdutch added a commit to Kingdutch/graphql-php that referenced this issue Nov 27, 2020
Kingdutch added a commit to Kingdutch/graphql-php that referenced this issue Nov 27, 2020
Kingdutch added a commit to Kingdutch/graphql-php that referenced this issue Nov 27, 2020
Kingdutch added a commit to Kingdutch/graphql-php that referenced this issue Nov 27, 2020
Kingdutch added a commit to Kingdutch/graphql-php that referenced this issue Nov 27, 2020
Kingdutch added a commit to Kingdutch/graphql-php that referenced this issue Nov 27, 2020
Kingdutch added a commit to Kingdutch/graphql-php that referenced this issue Nov 27, 2020
@Kingdutch
Copy link
Contributor Author

BuildClientSchema::testLegacySupportForInterfacesWithNullAsInterfacesField is part of making sure this change is backwards compatible : ) From what I can tell, the public API also hasn't changed. So pending a maintainer review I think this should be able to land in a minor version.

The only thing I can come up with is if people made their own classes to implement types that do not extend ObjectType or InterfaceType but should support implementing interfaces. Those custom classes should implement the ImplementingInterface interface (which both ObjectType and InterfaceType do in the PR). I don't really see a use-case for this though as the library assumes specific types in many locations.

vladar pushed a commit that referenced this issue Jan 23, 2021
* Implement support for interfaces implementing interfaces

Closes #728

* Implement tests for interfaces implementing interfaces

This ports the JavaScript tests for `RFC: Allow interfaces to
implement other interfaces` to PHP. This should ensure that there is
sufficient test coverage for the changes made to support interfaces
implementing interfaces.

Tests taken from https://github.com/graphql/graphql-js/pull/2084/files
including any typoes in test description strings to aid in comparison.

* Fix extend implement interface in Parser

This is part of the update to allow interfaces to implement interfaces.
A single extend statement to add an implementation of an interface
without field declarations is valid. This was caught by tests and brings
in a change from graphql/graphql-js#2084

* Validate interface implemented ancestors

Part of the work done to implement interfaces implementing interfaces.
This was caught by test and improves on the previously done changes for
the SchemaValidationContext by implementing
`validateTypeImplementsAncestors` which was missing.

* Properly apply Schema changes for interface extension support

This redoes the work done for the Schema class since it was previously
guessed at. It now more closely follows graphql/graphql-js/pull/2084

* Improve interface extension related typehints

Co-authored-by: Benedikt Franke <[email protected]>

* Refine types

* Remove complex array shape type

* Don't remove but deprecate DANGEROUS_CHANGE_IMPLEMENTED_INTERFACE_ADDED

Removing a public constant is a breaking change and can not be
implemented in a minor version. Instead the internal value is changed to
ensure that existing code keeps working with the new interface
implementation logic.

* Don't remove but deprecate BREAKING_CHANGE_INTERFACE_REMOVED_FROM_OBJECT

Co-authored-by: Benedikt Franke <[email protected]>
Co-authored-by: Benedikt Franke <[email protected]>
LastDragon-ru added a commit to LastDragon-ru/lara-asp that referenced this issue Jan 30, 2022
…d advanced formating.

These are few known issues:
* Schema description is not yet supported by `graphql-php` (webonyx/graphql-php#1027)
* "implements" for interfaces is not yet supported by Lighthouse (just for the reference webonyx/graphql-php#728)
LastDragon-ru added a commit to LastDragon-ru/lara-asp-core that referenced this issue Jan 30, 2022
…d advanced formating.

These are few known issues:
* Schema description is not yet supported by `graphql-php` (webonyx/graphql-php#1027)
* "implements" for interfaces is not yet supported by Lighthouse (just for the reference webonyx/graphql-php#728)
LastDragon-ru added a commit to LastDragon-ru/lara-asp-graphql that referenced this issue Jan 30, 2022
…d advanced formating.

These are few known issues:
* Schema description is not yet supported by `graphql-php` (webonyx/graphql-php#1027)
* "implements" for interfaces is not yet supported by Lighthouse (just for the reference webonyx/graphql-php#728)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants