Types should implement the same required fields as ThingInterface
Closed this issue · 5 comments
In PR #84 we've made some fields optional. However, the CE-API returns an error when using an interface fragment on a type with conflicting properties.
The conflict (required vs optional):
ce-api/src/schema/interface/ThingInterface.graphql
Lines 19 to 20 in 2f110d4
ce-api/src/schema/type/Person.graphql
Lines 51 to 52 in 2f110d4
The query:
fragment person on ThingInterface {
name
}
query {
Person {
name
...person
}
}
The error:
Fields \"name\" conflict because they return conflicting types String! and String. Use different aliases on the fields to fetch both if this was intentional.
From my comment in the original PR:
I only made name optional in the types that we're using at the moment, because I wasn't sure of the effect of making it optional in some of the other types.
So it looks like it is necessary to make it optional in ThingInterface and in all subclasses of this.
However, because types like ControlAction are also a Thing, this makes name optional here.
@musicog I looked at the definition at https://schema.org/name and https://schema.org/ControlAction and it seems that there is nothing in schema.org that defines that a field is required. Is that right? In this case, I think we should just make name optional everywhere.
@alastair I've found this comment stating that schema.org doesn't dictate fields that are required.
schemaorg/schemaorg#1177 (comment)
If schema.org doesn't define which fields are required, I guess we should define the required fields. However, the important part is that if this is a field on ThingInterface, it should be changed in all types implementing that interface as well.
Great, thanks for finding this.
As we have a clear requirement for having names optional, I will make a PR that changes all name fields on types that inherit from ThingInterface to be optional.
In our case, ThingInterface and ActionInterface both have a name field, and types such as Action and ControlAction inherit from both of them.
One option would be to require people to use fragments on ActionInterface when getting data from Action/ControlAction, instead of ThingInterface. This would allow us to keep these items required.
Or for simplicity do you think it'd be easier to just make everything optional?
Sorry, I didn't look carefully enough, ActionInterface doesn't define name.